Fixed bug in sorting in findKth function

jabley · Aug 17, 2010 · 1a9109d · 1a9109d
1 parent e4b13a4
commit 1a9109d
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 11 deletions.
diff --git a/src/java/voldemort/client/protocol/admin/AdminClient.java b/src/java/voldemort/client/protocol/admin/AdminClient.java
@@ -1198,7 +1198,7 @@ public String fetchStore(int nodeId,
         VAdminProto.FetchStoreRequest.Builder fetchStoreRequest = VAdminProto.FetchStoreRequest.newBuilder()
                                                                                                .setStoreName(storeName)
                                                                                                .setStoreDir(storeDir);
-        if(pushVersion >= 0) {
+        if(pushVersion > 0) {
             fetchStoreRequest.setPushVersion(pushVersion);
         }
 

diff --git a/src/java/voldemort/store/readonly/ReadOnlyUtils.java b/src/java/voldemort/store/readonly/ReadOnlyUtils.java
@@ -116,11 +116,20 @@ public int compare(File file1, File file2) {
                 long fileId1 = getVersionId(file1), fileId2 = getVersionId(file2);
                 if(fileId1 == fileId2) {
                     return 0;
-                } else if(fileId1 < fileId2) {
-                    return -1;
                 } else {
-                    return 1;
+                    if(fileId1 == -1) {
+                        return 1;
+                    }
+                    if(fileId2 == -1) {
+                        return -1;
+                    }
+                    if(fileId1 < fileId2) {
+                        return -1;
+                    } else {
+                        return 1;
+                    }
                 }
+
             }
         });
 

diff --git a/src/java/voldemort/store/readonly/swapper/HttpStoreSwapper.java b/src/java/voldemort/store/readonly/swapper/HttpStoreSwapper.java
@@ -49,7 +49,7 @@ public String call() throws Exception {
                     String storeDir = basePath + "/node-" + node.getId();
                     post.addParameter("dir", storeDir);
                     post.addParameter("store", storeName);
-                    if(pushVersion >= 0)
+                    if(pushVersion > 0)
                         post.addParameter("pushVersion", Long.toString(pushVersion));
                     logger.info("Invoking fetch for node " + node.getId() + " for " + storeDir);
                     int responseCode = httpClient.executeMethod(post);

diff --git a/test/unit/voldemort/store/readonly/ReadOnlyStorageEngineTest.java b/test/unit/voldemort/store/readonly/ReadOnlyStorageEngineTest.java
@@ -356,7 +356,7 @@ public void testBadSwapDataThrows() throws IOException {
         ReadOnlyStorageEngine engine = new ReadOnlyStorageEngine("test", strategy, dir, 2);
         assertVersionsExist(dir, 0);
 
-        // swap to a directory with bad name
+        // swap to a directory with bad data
         File newDir = new File(dir, "version-1");
         createStoreFiles(newDir, 73, 1024, 2);
         engine.swapFiles(newDir.getAbsolutePath());

diff --git a/test/unit/voldemort/store/readonly/ReadOnlyUtilsTest.java b/test/unit/voldemort/store/readonly/ReadOnlyUtilsTest.java
@@ -68,34 +68,42 @@ public void testVersionParsing() throws IOException {
         assertEquals(ReadOnlyUtils.getVersionDirs(tempParentDir).length, 1);
     }
 
-    private void generateVersionDirs(final File parentDir, final int... versions) {
+    private void generateVersionDirs(final File parentDir, final String... versions) {
         File versionDir = null;
-        for(int version: versions) {
+        for(String version: versions) {
             versionDir = new File(parentDir, "version-" + version);
             Utils.mkdirs(versionDir);
         }
     }
 
     public void testFindKthVersionedDir() {
         File tempParentDir = TestUtils.createTempDir();
-        generateVersionDirs(tempParentDir, 0);
+        generateVersionDirs(tempParentDir, "0");
         assertNull(ReadOnlyUtils.findKthVersionedDir(ReadOnlyUtils.getVersionDirs(tempParentDir), 2));
         assertEquals(new File(tempParentDir, "version-0"),
                      ReadOnlyUtils.findKthVersionedDir(ReadOnlyUtils.getVersionDirs(tempParentDir),
                                                        1));
 
         tempParentDir = TestUtils.createTempDir();
-        int[] versions = { 6, 10, 20, 100, 200, 250, 300 };
+        String[] versions = { "6", "10", "20", "100", "200", "250", "300" };
         generateVersionDirs(tempParentDir, versions);
+
         assertNull(ReadOnlyUtils.findKthVersionedDir(ReadOnlyUtils.getVersionDirs(tempParentDir),
                                                      -1));
         assertNull(ReadOnlyUtils.findKthVersionedDir(ReadOnlyUtils.getVersionDirs(tempParentDir), 8));
         int numVersion = 1;
-        for(int version: versions) {
+        for(String version: versions) {
             assertEquals(new File(tempParentDir, "version-" + version),
                          ReadOnlyUtils.findKthVersionedDir(ReadOnlyUtils.getVersionDirs(tempParentDir),
                                                            numVersion++));
         }
 
+        tempParentDir = TestUtils.createTempDir();
+        String[] version2 = { "6", "10", "6a" };
+        generateVersionDirs(tempParentDir, version2);
+        assertEquals(new File(tempParentDir, "version-6a"),
+                     ReadOnlyUtils.findKthVersionedDir(ReadOnlyUtils.getVersionDirs(tempParentDir),
+                                                       3));
+
     }
 }