Browse files

Cleaned up comments and TODOs from prior commits.

  • Loading branch information...
1 parent 2411377 commit 61c11c7edf1472ae4c6b7323b7790356b5aa7e8b @jayjwylie jayjwylie committed Oct 15, 2012
View
2 src/java/voldemort/server/VoldemortServer.java
@@ -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;
View
42 test/common/voldemort/ServerTestUtils.java
@@ -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.");
@@ -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();
@@ -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,
View
5 test/unit/voldemort/utils/ServerTestUtilsTest.java
@@ -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 },
@@ -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.