Skip to content

Commit

Permalink
Cleaned up comments and TODOs from prior commits.
Browse files Browse the repository at this point in the history
  • Loading branch information
jayjwylie committed Oct 16, 2012
1 parent 2411377 commit 61c11c7
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 6 deletions.
2 changes: 0 additions & 2 deletions src/java/voldemort/server/VoldemortServer.java
Expand Up @@ -91,8 +91,6 @@ public VoldemortServer(VoldemortConfig config) {
this.services = createServices();
}

// TODO: Should we use a different VoldemortServer construction depending on
// whether cluster is written previously!?!?! (cluster.xml issue)
public VoldemortServer(VoldemortConfig config, Cluster cluster) {
super(ServiceType.VOLDEMORT);
this.voldemortConfig = config;
Expand Down
42 changes: 39 additions & 3 deletions test/common/voldemort/ServerTestUtils.java
Expand Up @@ -228,14 +228,22 @@ public static HttpStore getHttpStore(String storeName,
}

/**
* Return a free port as chosen by new ServerSocket(0)
* Return a free port as chosen by new ServerSocket(0).
*
* There is no guarantee that the port returned will be free when the caller
* attempts to bind to the port. This is a time-of-check-to-time-of-use
* (TOCTOU) issue that cannot be avoided.
*/
public static int findFreePort() {
return findFreePorts(1)[0];
}

/**
* Return an array of free ports as chosen by new ServerSocket(0)
*
* There is no guarantee that the ports returned will be free when the
* caller attempts to bind to some returned port. This is a
* time-of-check-to-time-of-use (TOCTOU) issue that cannot be avoided.
*/
public static int[] findFreePorts(int n) {
logger.info("findFreePorts cannot guarantee that ports identified as free will still be free when used. This is effectively a TOCTOU issue. Expect intermittent BindException when \"free\" ports are used.");
Expand Down Expand Up @@ -669,8 +677,17 @@ public static VoldemortServer startVoldemortServer(SocketStoreFactory socketStor
VoldemortConfig config,
Cluster cluster) {

// TODO: Should this use VoldemortServer(config) instead!?!?!?
// (config.xml)
// TODO: Some tests that use this method fail intermittently with the
// following output:
//
// A successor version version() to this version() exists for key
// cluster.xml
// voldemort.versioning.ObsoleteVersionException: A successor version
// version() to this version() exists for key cluster.xml"
//
// Need to trace through the constructor VoldemortServer(VoldemortConfig
// config, Cluster cluster) to understand how this error is possible,
// and why it only happens intermittently.
VoldemortServer server = new VoldemortServer(config, cluster);
server.start();

Expand Down Expand Up @@ -742,6 +759,25 @@ protected static Cluster internalStartVoldemortCluster(int numServers,
return cluster;
}

/**
* This method wraps up work that is done in many different tests to set up
* some number of Voldemort servers in a cluster. This method masks an
* intermittent TOCTOU problem with the ports identified by
* {@link #findFreePorts(int)} not actually being free when a server needs
* to bind to them.
*
* @param numServers
* @param voldemortServers
* @param partitionMap
* @param socketStoreFactory
* @param useNio
* @param clusterFile
* @param storeFile
* @param properties
* @return Cluster object that was used to successfully start all of the
* servers.
* @throws IOException
*/
public static Cluster startVoldemortCluster(int numServers,
VoldemortServer[] voldemortServers,
int[][] partitionMap,
Expand Down
5 changes: 4 additions & 1 deletion test/unit/voldemort/utils/ServerTestUtilsTest.java
Expand Up @@ -62,7 +62,9 @@ public void startMultipleVoldemortClusters() throws IOException {
}
}

// **********************************************************************
// * START : TESTS THAT HELPED FIND ROOT CAUSE OF BindException PROBLEM *

// @Test
public void startMultipleVoldemortServers() throws IOException {
Cluster cluster = ServerTestUtils.getLocalCluster(8, new int[][] { { 0 }, { 1 }, { 2 },
Expand Down Expand Up @@ -177,6 +179,7 @@ public void testFindFreePorts100() throws Exception {
testFindFreePorts();
}
}
// ** END : TESTS THAT HELPED FIND ROOT CAUSE OF BindException PROBLEM **

// ** END : TESTS THAT HELPED FIND ROOT CAUSE OF BindException PROBLEM **
// **********************************************************************
}

0 comments on commit 61c11c7

Please sign in to comment.