You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by iv...@apache.org on 2012/12/12 18:27:02 UTC
svn commit: r1420854 - in /zookeeper/bookkeeper/trunk: ./
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/
Author: ivank
Date: Wed Dec 12 17:26:59 2012
New Revision: 1420854
URL: http://svn.apache.org/viewvc?rev=1420854&view=rev
Log:
BOOKKEEPER-493: moveLedgerIndexFile might have chance pickup same directory (sijie via ivank)
Added:
zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestLedgerDirsManager.java
Modified:
zookeeper/bookkeeper/trunk/CHANGES.txt
zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java
zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java
Modified: zookeeper/bookkeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1420854&r1=1420853&r2=1420854&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/CHANGES.txt (original)
+++ zookeeper/bookkeeper/trunk/CHANGES.txt Wed Dec 12 17:26:59 2012
@@ -134,6 +134,8 @@ Trunk (unreleased changes)
BOOKKEEPER-497: GcLedgersTest has a potential race (ivank via sijie)
+ BOOKKEEPER-493: moveLedgerIndexFile might have chance pickup same directory (sijie via ivank)
+
hedwig-protocol:
BOOKKEEPER-394: CompositeException message is not useful (Stu Hood via sijie)
Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java?rev=1420854&r1=1420853&r2=1420854&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCacheImpl.java Wed Dec 12 17:26:59 2012
@@ -253,7 +253,7 @@ public class LedgerCacheImpl implements
if (masterKey == null) {
throw new Bookie.NoLedgerException(ledger);
}
- lf = getNewLedgerIndexFile(ledger);
+ lf = getNewLedgerIndexFile(ledger, null);
// A new ledger index file has been created for this Bookie.
// Add this new ledger to the set of active ledgers.
LOG.debug("New ledger index file created for ledgerId: {}", ledger);
@@ -261,8 +261,7 @@ public class LedgerCacheImpl implements
}
evictFileInfoIfNecessary();
fi = new FileInfo(lf, masterKey);
- if (ledgerDirsManager.isDirFull(lf.getParentFile()
- .getParentFile().getParentFile())) {
+ if (ledgerDirsManager.isDirFull(getLedgerDirForLedger(fi))) {
moveLedgerIndexFile(ledger, fi);
}
fileInfoCache.put(ledger, fi);
@@ -275,8 +274,19 @@ public class LedgerCacheImpl implements
}
}
- private File getNewLedgerIndexFile(Long ledger) throws NoWritableLedgerDirException {
- File dir = ledgerDirsManager.pickRandomWritableDir();
+ /**
+ * Get a new index file for ledger excluding directory <code>excludedDir</code>.
+ *
+ * @param ledger
+ * Ledger id.
+ * @param excludedDir
+ * The ledger directory to exclude.
+ * @return new index file object.
+ * @throws NoWritableLedgerDirException if there is no writable dir available.
+ */
+ private File getNewLedgerIndexFile(Long ledger, File excludedDir)
+ throws NoWritableLedgerDirException {
+ File dir = ledgerDirsManager.pickRandomWritableDir(excludedDir);
String ledgerName = getLedgerName(ledger);
return new File(dir, ledgerName);
}
@@ -349,7 +359,7 @@ public class LedgerCacheImpl implements
// open index files to new location
for (Long l : dirtyLedgers) {
FileInfo fi = getFileInfo(l, null);
- File currentDir = fi.getLf().getParentFile().getParentFile().getParentFile();
+ File currentDir = getLedgerDirForLedger(fi);
if (ledgerDirsManager.isDirFull(currentDir)) {
moveLedgerIndexFile(l, fi);
}
@@ -377,8 +387,18 @@ public class LedgerCacheImpl implements
}
}
+ /**
+ * Get the ledger directory that the ledger index belongs to.
+ *
+ * @param fi File info of a ledger
+ * @return ledger directory that the ledger belongs to.
+ */
+ private File getLedgerDirForLedger(FileInfo fi) {
+ return fi.getLf().getParentFile().getParentFile().getParentFile();
+ }
+
private void moveLedgerIndexFile(Long l, FileInfo fi) throws NoWritableLedgerDirException, IOException {
- File newLedgerIndexFile = getNewLedgerIndexFile(l);
+ File newLedgerIndexFile = getNewLedgerIndexFile(l, getLedgerDirForLedger(fi));
fi.moveToNewLocation(newLedgerIndexFile, fi.getSizeSinceLastwrite());
}
Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java?rev=1420854&r1=1420853&r2=1420854&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java (original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java Wed Dec 12 17:26:59 2012
@@ -115,8 +115,35 @@ public class LedgerDirsManager {
* Returns one of the ledger dir from writable dirs list randomly.
*/
File pickRandomWritableDir() throws NoWritableLedgerDirException {
+ return pickRandomWritableDir(null);
+ }
+
+ /**
+ * Pick up a writable dir from available dirs list randomly. The <code>excludedDir</code>
+ * will not be pickedup.
+ *
+ * @param excludedDir
+ * The directory to exclude during pickup.
+ * @throws NoWritableLedgerDirException if there is no writable dir available.
+ */
+ File pickRandomWritableDir(File excludedDir) throws NoWritableLedgerDirException {
List<File> writableDirs = getWritableLedgerDirs();
- return writableDirs.get(rand.nextInt(writableDirs.size()));
+
+ final int start = rand.nextInt(writableDirs.size());
+ int idx = start;
+ File candidate = writableDirs.get(idx);
+ while (null != excludedDir && excludedDir.equals(candidate)) {
+ idx = (idx + 1) % writableDirs.size();
+ if (idx == start) {
+ // after searching all available dirs,
+ // no writable dir is found
+ throw new NoWritableLedgerDirException("No writable directories found from "
+ + " available writable dirs (" + writableDirs + ") : exclude dir "
+ + excludedDir);
+ }
+ candidate = writableDirs.get(idx);
+ }
+ return candidate;
}
public void addLedgerDirsListener(LedgerDirsListener listener) {
Added: zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestLedgerDirsManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestLedgerDirsManager.java?rev=1420854&view=auto
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestLedgerDirsManager.java (added)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestLedgerDirsManager.java Wed Dec 12 17:26:59 2012
@@ -0,0 +1,80 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie;
+
+import java.io.File;
+
+import junit.framework.TestCase;
+
+import org.apache.bookkeeper.bookie.LedgerDirsManager.NoWritableLedgerDirException;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TestLedgerDirsManager extends TestCase {
+ static Logger LOG = LoggerFactory.getLogger(TestLedgerDirsManager.class);
+
+ ServerConfiguration conf = new ServerConfiguration();
+ File curDir;
+ LedgerDirsManager dirsManager;
+
+ @Before
+ public void setUp() throws Exception {
+ File tmpDir = File.createTempFile("bkTest", ".dir");
+ tmpDir.delete();
+ tmpDir.mkdir();
+ curDir = Bookie.getCurrentDirectory(tmpDir);
+ Bookie.checkDirectoryStructure(curDir);
+
+ ServerConfiguration conf = new ServerConfiguration();
+ conf.setLedgerDirNames(new String[] {tmpDir.toString()});
+
+ dirsManager = new LedgerDirsManager(conf);
+ }
+
+ @Test
+ public void testPickWritableDirExclusive() throws Exception {
+ try {
+ dirsManager.pickRandomWritableDir(curDir);
+ fail("Should not reach here due to there is no writable ledger dir.");
+ } catch (NoWritableLedgerDirException nwlde) {
+ // expected to fail with no writable ledger dir
+ assertTrue(true);
+ }
+ }
+
+ @Test
+ public void testNoWritableDir() throws Exception {
+ try {
+ dirsManager.addToFilledDirs(curDir);
+ dirsManager.pickRandomWritableDir();
+ fail("Should not reach here due to there is no writable ledger dir.");
+ } catch (NoWritableLedgerDirException nwlde) {
+ // expected to fail with no writable ledger dir
+ assertEquals("Should got NoWritableLedgerDirException w/ 'All ledger directories are non writable'.",
+ "All ledger directories are non writable", nwlde.getMessage());
+ }
+ }
+
+}