You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by si...@apache.org on 2018/10/28 13:06:22 UTC
[bookkeeper] branch master updated: Kill
LedgerMetadata#isConflictWith
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 fcd63f9 Kill LedgerMetadata#isConflictWith
fcd63f9 is described below
commit fcd63f9a78a59aa48c693b4491fc5241ed350ff6
Author: Ivan Kelly <iv...@apache.org>
AuthorDate: Sun Oct 28 14:06:18 2018 +0100
Kill LedgerMetadata#isConflictWith
It doesn't make sense anymore, as local copies of the metadata are
never modified, only updated with the latest version from the metadata
store.
In effect, this logic has been broken out to the places where we try
to update the metadata store copy. Each time we try to update the
metadata store, we ensure that the update we are applying still makes
sense with regard to the copy of the metadata we are updating.
Master issue: #281
Reviewers: Sijie Guo <si...@apache.org>, Enrico Olivelli <eo...@gmail.com>
This closes #1760 from ivankelly/kill-is-conflict-with
---
.../apache/bookkeeper/client/LedgerMetadata.java | 61 ----------------------
.../bookkeeper/client/LedgerMetadataTest.java | 56 --------------------
.../bookkeeper/client/UpdateLedgerCmdTest.java | 3 --
.../bookkeeper/client/UpdateLedgerOpTest.java | 9 ----
4 files changed, 129 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java
index 02e0ede..d8e1c1b 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java
@@ -34,7 +34,6 @@ import java.nio.CharBuffer;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -672,66 +671,6 @@ public class LedgerMetadata implements org.apache.bookkeeper.client.api.LedgerMe
return true;
}
- /**
- * Is the metadata conflict with new updated metadata.
- *
- * @param newMeta
- * Re-read metadata
- * @return true if the metadata is conflict.
- */
- boolean isConflictWith(LedgerMetadata newMeta) {
- /*
- * if length & close have changed, then another client has
- * opened the ledger, can't resolve this conflict.
- */
-
- if (metadataFormatVersion != newMeta.metadataFormatVersion
- || ensembleSize != newMeta.ensembleSize
- || writeQuorumSize != newMeta.writeQuorumSize
- || ackQuorumSize != newMeta.ackQuorumSize
- || length != newMeta.length
- || state != newMeta.state
- || !digestType.equals(newMeta.digestType)
- || !Arrays.equals(password, newMeta.password)
- || !LedgerMetadata.areByteArrayValMapsEqual(customMetadata, newMeta.customMetadata)) {
- return true;
- }
-
- // verify the ctime
- if (storeSystemtimeAsLedgerCreationTime != newMeta.storeSystemtimeAsLedgerCreationTime) {
- return true;
- } else if (storeSystemtimeAsLedgerCreationTime) {
- return ctime != newMeta.ctime;
- }
-
- if (state == LedgerMetadataFormat.State.CLOSED
- && lastEntryId != newMeta.lastEntryId) {
- return true;
- }
- // if ledger is closed, we can just take the new ensembles
- if (newMeta.state != LedgerMetadataFormat.State.CLOSED) {
- // allow new metadata to be one ensemble less than current metadata
- // since ensemble change might kick in when recovery changed metadata
- int diff = ensembles.size() - newMeta.ensembles.size();
- if (0 != diff && 1 != diff) {
- return true;
- }
- // ensemble distribution should be same
- // we don't check the detail ensemble, since new bookie will be set
- // using recovery tool.
- Iterator<Long> keyIter = ensembles.keySet().iterator();
- Iterator<Long> newMetaKeyIter = newMeta.ensembles.keySet().iterator();
- for (int i = 0; i < newMeta.ensembles.size(); i++) {
- Long curKey = keyIter.next();
- Long newMetaKey = newMetaKeyIter.next();
- if (!curKey.equals(newMetaKey)) {
- return true;
- }
- }
- }
- return false;
- }
-
@Override
public String toString() {
return toStringRepresentation(true);
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerMetadataTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerMetadataTest.java
index dc947dd..5f802cb 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerMetadataTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerMetadataTest.java
@@ -100,62 +100,6 @@ public class LedgerMetadataTest {
}
@Test
- public void testIsConflictWithStoreSystemtimeAsLedgerCtimeDisabled() {
- LedgerMetadata lm1 = new LedgerMetadata(
- 3,
- 3,
- 2,
- DigestType.CRC32,
- passwd,
- Collections.emptyMap(),
- false);
- LedgerMetadata lm2 = new LedgerMetadata(lm1);
-
- lm1.setCtime(1L);
- lm2.setCtime(2L);
- assertFalse(lm1.isConflictWith(lm2));
- }
-
- @Test
- public void testIsConflictWithStoreSystemtimeAsLedgerCtimeEnabled() {
- LedgerMetadata lm1 = new LedgerMetadata(
- 3,
- 3,
- 2,
- DigestType.CRC32,
- passwd,
- Collections.emptyMap(),
- true);
- LedgerMetadata lm2 = new LedgerMetadata(lm1);
-
- lm1.setCtime(1L);
- lm2.setCtime(2L);
- assertTrue(lm1.isConflictWith(lm2));
- }
-
- @Test
- public void testIsConflictWithDifferentStoreSystemtimeAsLedgerCtimeFlags() {
- LedgerMetadata lm1 = new LedgerMetadata(
- 3,
- 3,
- 2,
- DigestType.CRC32,
- passwd,
- Collections.emptyMap(),
- true);
- LedgerMetadata lm2 = new LedgerMetadata(
- 3,
- 3,
- 2,
- DigestType.CRC32,
- passwd,
- Collections.emptyMap(),
- false);
-
- assertTrue(lm1.isConflictWith(lm2));
- }
-
- @Test
public void testToString() {
LedgerMetadata lm1 = new LedgerMetadata(
3,
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerCmdTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerCmdTest.java
index 6eca0de..fa18497 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerCmdTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerCmdTest.java
@@ -97,9 +97,6 @@ public class UpdateLedgerCmdTest extends BookKeeperClusterTestCase {
List<BookieSocketAddress> ensemble;
int updatedLedgersCount = 0;
for (LedgerHandle lh : ledgers) {
- // ledger#close() would hit BadVersion exception as rename
- // increments cversion. But LedgerMetadata#isConflictWith()
- // gracefully handles this conflicts.
lh.close();
LedgerHandle openLedger = bk.openLedger(lh.getId(), digestType, PASSWORD.getBytes());
ensemble = openLedger.getLedgerMetadata().getEnsemble(0);
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerOpTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerOpTest.java
index 739abea..ad5c450 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerOpTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerOpTest.java
@@ -110,9 +110,6 @@ public class UpdateLedgerOpTest extends BookKeeperClusterTestCase {
updateLedgerOp.updateBookieIdInLedgers(curBookieAddr, toBookieAddr, 5, Integer.MIN_VALUE, progressable);
for (LedgerHandle lh : ledgers) {
- // ledger#close() would hit BadVersion exception as rename
- // increments cversion. But LedgerMetadata#isConflictWith()
- // gracefully handles this conflicts.
lh.close();
LedgerHandle openLedger = bk.openLedger(lh.getId(), digestType, PASSWORD.getBytes());
ensemble = openLedger.getLedgerMetadata().getEnsemble(0);
@@ -218,9 +215,6 @@ public class UpdateLedgerOpTest extends BookKeeperClusterTestCase {
bsConfs.add(serverConf1);
bs.add(startBookie(serverConf1));
- // ledger#asyncAddEntry() would hit BadVersion exception as rename incr
- // cversion. But LedgerMetadata#isConflictWith() gracefully handles
- // this conflicts.
final CountDownLatch latch = new CountDownLatch(1);
final AtomicInteger rc = new AtomicInteger(BKException.Code.OK);
lh.asyncAddEntry("foobar".getBytes(), new AddCallback() {
@@ -304,9 +298,6 @@ public class UpdateLedgerOpTest extends BookKeeperClusterTestCase {
List<BookieSocketAddress> ensemble;
int updatedLedgersCount = 0;
for (LedgerHandle lh : ledgers) {
- // ledger#close() would hit BadVersion exception as rename
- // increments cversion. But LedgerMetadata#isConflictWith()
- // gracefully handles this conflicts.
lh.close();
LedgerHandle openLedger = bk.openLedger(lh.getId(), digestType, PASSWORD.getBytes());
ensemble = openLedger.getLedgerMetadata().getEnsemble(0);