This is an automated email from the ASF dual-hosted git repository.
sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new c49621b ISSUE #208: Improve ledger fence logic
c49621b is described below
commit c49621bacaa960d240afbbee83a9703b4cbc3ec2
Author: Sijie Guo <[email protected]>
AuthorDate: Mon Jul 17 13:47:37 2017 -0700
ISSUE #208: Improve ledger fence logic
Descriptions of the changes in this PR:
Problem:
When bookie receive a fence request and couldn't find any writable dirs
for the new index file, it will throw exception. This behavior can be improved,
because as long as ledger fence request be persisted in Journal, we can say the
fence request succeed. It should not depends on the success of flushing new
index file.
Solution:
- Add option to fall back to pick from all directories regardless of
writable or not when we getFileInfo for a new ledger
- Return success only when ledger fence request has been persisted in
Journal
---
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
- [x] Make sure the PR title is formatted like:
`<Issue #>: Description of pull request`
`e.g. Issue 123: Description ...`
- [x] Make sure tests pass via `mvn clean apache-rat:check install
findbugs:check`.
- [x] Replace `<Issue #>` in the title with the actual Issue number, if
there is one.
---
Author: Sijie Guo <[email protected]>
Author: Yiming Zang <[email protected]>
Reviewers: Enrico Olivelli <[email protected]>, Jia Zhai <None>
This closes #205 from sijie/improve_fencing_behavior, closes #208
---
.../java/org/apache/bookkeeper/bookie/Bookie.java | 24 +--------
.../bookkeeper/bookie/IndexPersistenceMgr.java | 35 ++++++++++++-
.../apache/bookkeeper/bookie/LedgerDescriptor.java | 20 +++++++-
.../bookkeeper/bookie/LedgerDescriptorImpl.java | 58 +++++++++++++++++++++-
.../bookkeeper/bookie/LedgerDirsManager.java | 11 +++-
5 files changed, 118 insertions(+), 30 deletions(-)
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
index 769bda1..3375204 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
@@ -1492,29 +1492,7 @@ public class Bookie extends BookieCriticalThread {
*/
public SettableFuture<Boolean> fenceLedger(long ledgerId, byte[]
masterKey) throws IOException, BookieException {
LedgerDescriptor handle = handles.getHandle(ledgerId, masterKey);
- boolean success;
- synchronized (handle) {
- success = handle.setFenced();
- }
- if (success) {
- // fenced first time, we should add the key to journal ensure we
can rebuild
- ByteBuffer bb = ByteBuffer.allocate(8 + 8);
- bb.putLong(ledgerId);
- bb.putLong(METAENTRY_ID_FENCE_KEY);
- bb.flip();
-
- FutureWriteCallback fwc = new FutureWriteCallback();
- if (LOG.isDebugEnabled()) {
- LOG.debug("record fenced state for ledger {} in journal.",
ledgerId);
- }
- getJournal(ledgerId).logAddEntry(bb, fwc, null);
- return fwc.getResult();
- } else {
- // already fenced
- SettableFuture<Boolean> successFuture = SettableFuture.create();
- successFuture.set(true);
- return successFuture;
- }
+ return handle.fenceAndLogInJournal(getJournal(ledgerId));
}
public ByteBuf readEntry(long ledgerId, long entryId)
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java
index 323ad62..9a34101 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java
@@ -125,7 +125,7 @@ public class IndexPersistenceMgr {
throw new Bookie.NoLedgerException(ledger);
}
// We don't have a ledger index file on disk, so create it.
- lf = getNewLedgerIndexFile(ledger, null);
+ lf = getNewLedgerIndexFile(ledger, null, true);
createdNewFile = true;
}
}
@@ -176,7 +176,38 @@ public class IndexPersistenceMgr {
*/
private File getNewLedgerIndexFile(Long ledger, File excludedDir)
throws NoWritableLedgerDirException {
- File dir =
ledgerDirsManager.pickRandomWritableDirForNewIndexFile(excludedDir);
+ return getNewLedgerIndexFile(ledger, excludedDir, false);
+ }
+
+ /**
+ * Get a new index file for a ledger in a lazy way.
+ *
+ + <p>If fallback is false, this function will throw exception when there
are no writable dirs.
+ + If fallback is true and there's no writable dirs, it will ignore the
error and pick any dir.
+ + Set fallback to true is useful when we want to delay disk check and
just get the File pointer, e.g. fence ledger
+ *
+ * @param ledger
+ * Ledger id.
+ * @param excludedDir
+ * The ledger directory to exclude.
+ * @param fallback
+ * If fallback is false, the function will throw exception when
there are no writable dirs;
+ * If it is true and there's no writable dirs, it will ignore the
error and pick any dir.
+ * @return new index file object.
+ * @throws NoWritableLedgerDirException if there is no writable dir
available.
+ */
+ private File getNewLedgerIndexFile(Long ledger, File excludedDir, boolean
fallback)
+ throws NoWritableLedgerDirException {
+ File dir = null;
+ try {
+ dir =
ledgerDirsManager.pickRandomWritableDirForNewIndexFile(excludedDir);
+ } catch (NoWritableLedgerDirException e) {
+ if (fallback) {
+ dir = ledgerDirsManager.pickRandomDir(excludedDir);
+ } else {
+ throw e;
+ }
+ }
String ledgerName = getLedgerName(ledger);
return new File(dir, ledgerName);
}
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptor.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptor.java
index 032dfe2..e562dcb 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptor.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptor.java
@@ -21,17 +21,21 @@
package org.apache.bookkeeper.bookie;
+import com.google.common.util.concurrent.SettableFuture;
import io.netty.buffer.ByteBuf;
-
+import io.netty.buffer.Unpooled;
import java.io.IOException;
import java.util.Observable;
import java.util.Observer;
+import static org.apache.bookkeeper.bookie.Bookie.METAENTRY_ID_FENCE_KEY;
+
/**
* Implements a ledger inside a bookie. In particular, it implements operations
* to write entries to a ledger and read entries from a ledger.
*/
public abstract class LedgerDescriptor {
+
static LedgerDescriptor create(byte[] masterKey,
long ledgerId,
LedgerStorage ledgerStorage) throws
IOException {
@@ -49,12 +53,26 @@ public abstract class LedgerDescriptor {
return new LedgerDescriptorReadOnlyImpl(ledgerId, ledgerStorage);
}
+ static ByteBuf createLedgerFenceEntry(Long ledgerId) {
+ ByteBuf bb = Unpooled.buffer(8 + 8);
+ bb.writeLong(ledgerId);
+ bb.writeLong(METAENTRY_ID_FENCE_KEY);
+ return bb;
+ }
+
abstract void checkAccess(byte masterKey[]) throws BookieException,
IOException;
abstract long getLedgerId();
abstract boolean setFenced() throws IOException;
abstract boolean isFenced() throws IOException;
+ /**
+ * When we fence a ledger, we need to first set ledger to fenced state in
memory and
+ * then log the fence entry in Journal so that we can rebuild the state.
+ *
+ * We should satisfy the future only after we complete logging fence entry
in Journal
+ */
+ abstract SettableFuture<Boolean> fenceAndLogInJournal(Journal journal)
throws IOException;
abstract long addEntry(ByteBuf entry) throws IOException;
abstract ByteBuf readEntry(long entryId) throws IOException;
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java
index c2246bf..c48cd75 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java
@@ -21,11 +21,13 @@
package org.apache.bookkeeper.bookie;
+import com.google.common.util.concurrent.SettableFuture;
import io.netty.buffer.ByteBuf;
import java.io.IOException;
import java.util.Arrays;
import java.util.Observable;
import java.util.Observer;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -38,10 +40,14 @@ public class LedgerDescriptorImpl extends LedgerDescriptor {
private final static Logger LOG =
LoggerFactory.getLogger(LedgerDescriptor.class);
final LedgerStorage ledgerStorage;
private long ledgerId;
-
final byte[] masterKey;
- LedgerDescriptorImpl(byte[] masterKey, long ledgerId, LedgerStorage
ledgerStorage) {
+ private AtomicBoolean fenceEntryPersisted = new AtomicBoolean();
+ private SettableFuture<Boolean> logFenceResult = null;
+
+ LedgerDescriptorImpl(byte[] masterKey,
+ long ledgerId,
+ LedgerStorage ledgerStorage) {
this.masterKey = masterKey;
this.ledgerId = ledgerId;
this.ledgerStorage = ledgerStorage;
@@ -81,6 +87,54 @@ public class LedgerDescriptorImpl extends LedgerDescriptor {
return ledgerStorage.getExplicitLac(ledgerId);
}
+ synchronized SettableFuture<Boolean> fenceAndLogInJournal(Journal journal)
throws IOException {
+ boolean success = this.setFenced();
+ if(success) {
+ // fenced for first time, we should add the key to journal ensure
we can rebuild.
+ return logFenceEntryInJournal(journal);
+ } else {
+ // If we reach here, it means the fence state in FileInfo has been
set (may not be persisted yet).
+ // However, writing the fence log entry to the journal might still
be in progress. This can happen
+ // when a bookie receives two fence requests almost at the same
time. The subsequent logic is used
+ // to check the fencing progress.
+ if(logFenceResult == null || fenceEntryPersisted.get()){
+ // Either ledger's fenced state is recovered from Journal
+ // Or Log fence entry in Journal succeed
+ SettableFuture<Boolean> result = SettableFuture.create();
+ result.set(true);
+ return result;
+ } else if (logFenceResult.isDone()) {
+ // We failed to log fence entry in Journal, try again.
+ return logFenceEntryInJournal(journal);
+ }
+ // Fencing is in progress
+ return logFenceResult;
+ }
+ }
+
+ /**
+ * Log the fence ledger entry in Journal so that we can rebuild the state.
+ * @param journal log the fence entry in the Journal
+ * @return A future which will be satisfied when add entry to journal
complete
+ */
+ private SettableFuture<Boolean> logFenceEntryInJournal(Journal journal) {
+ SettableFuture<Boolean> result;
+ synchronized (this) {
+ result = logFenceResult = SettableFuture.create();
+ }
+ ByteBuf entry = createLedgerFenceEntry(ledgerId);
+ journal.logAddEntry(entry, (rc, ledgerId, entryId, addr, ctx) -> {
+ LOG.debug("Record fenced state for ledger {} in journal with rc
{}", ledgerId, rc);
+ if (rc == 0) {
+ fenceEntryPersisted.compareAndSet(false, true);
+ result.set(true);
+ } else {
+ result.set(false);
+ }
+ }, null);
+ return result;
+ }
+
@Override
long addEntry(ByteBuf entry) throws IOException {
long ledgerId = entry.getLong(entry.readerIndex());
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java
index 23726ca..ad0777e 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java
@@ -325,8 +325,15 @@ public class LedgerDirsManager {
}
return pickRandomDir(writableDirsForNewIndexFile, excludedDir);
}
-
- File pickRandomDir(List<File> dirs, File excludedDir) throws
NoWritableLedgerDirException{
+
+ /**
+ * Return one dir from all dirs, regardless writable or not.
+ */
+ File pickRandomDir(File excludedDir) throws NoWritableLedgerDirException {
+ return pickRandomDir(getAllLedgerDirs(), excludedDir);
+ }
+
+ File pickRandomDir(List<File> dirs, File excludedDir) throws
NoWritableLedgerDirException {
final int start = rand.nextInt(dirs.size());
int idx = start;
File candidate = dirs.get(idx);
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
