You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2017/11/16 12:54:55 UTC
[GitHub] jiazhai closed pull request #730: Issue 726: Unit Tests failure with BKMetadataVersionException
jiazhai closed pull request #730: Issue 726: Unit Tests failure with BKMetadataVersionException
URL: https://github.com/apache/bookkeeper/pull/730
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 d75754284..44c8ce7d0 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
@@ -98,7 +98,13 @@ public LedgerMetadata(int ensembleSize,
this.ensembleSize = ensembleSize;
this.writeQuorumSize = writeQuorumSize;
this.ackQuorumSize = ackQuorumSize;
- this.ctime = System.currentTimeMillis();
+ if (storeSystemtimeAsLedgerCreationTime) {
+ this.ctime = System.currentTimeMillis();
+ } else {
+ // if client disables storing its system time as ledger creation time, there should be no ctime at this
+ // moment.
+ this.ctime = -1L;
+ }
this.storeSystemtimeAsLedgerCreationTime = storeSystemtimeAsLedgerCreationTime;
/*
@@ -119,6 +125,10 @@ public LedgerMetadata(int ensembleSize,
}
}
+ /**
+ * Used for testing purpose only.
+ */
+ @VisibleForTesting
public LedgerMetadata(int ensembleSize, int writeQuorumSize, int ackQuorumSize,
BookKeeper.DigestType digestType, byte[] password) {
this(ensembleSize, writeQuorumSize, ackQuorumSize, digestType, password, null, false);
@@ -130,7 +140,6 @@ public LedgerMetadata(int ensembleSize, int writeQuorumSize, int ackQuorumSize,
LedgerMetadata(LedgerMetadata other) {
this.ensembleSize = other.ensembleSize;
this.writeQuorumSize = other.writeQuorumSize;
- this.ctime = other.ctime;
this.ackQuorumSize = other.ackQuorumSize;
this.length = other.length;
this.lastEntryId = other.lastEntryId;
@@ -139,6 +148,8 @@ public LedgerMetadata(int ensembleSize, int writeQuorumSize, int ackQuorumSize,
this.version = other.version;
this.hasPassword = other.hasPassword;
this.digestType = other.digestType;
+ this.ctime = other.ctime;
+ this.storeSystemtimeAsLedgerCreationTime = other.storeSystemtimeAsLedgerCreationTime;
this.password = new byte[other.password.length];
System.arraycopy(other.password, 0, this.password, 0, other.password.length);
// copy the ensembles
@@ -178,6 +189,10 @@ public int getWriteQuorumSize() {
return writeQuorumSize;
}
+ public int getAckQuorumSize() {
+ return ackQuorumSize;
+ }
+
/**
* Get the creation timestamp of the ledger
* @return
@@ -186,8 +201,9 @@ public long getCtime() {
return ctime;
}
- public int getAckQuorumSize() {
- return ackQuorumSize;
+ @VisibleForTesting
+ void setCtime(long ctime) {
+ this.ctime = ctime;
}
/**
@@ -430,8 +446,10 @@ public static LedgerMetadata parseConfig(byte[] bytes, Version version, Optional
lc.writeQuorumSize = data.getQuorumSize();
if (data.hasCtime()) {
lc.ctime = data.getCtime();
+ lc.storeSystemtimeAsLedgerCreationTime = true;
} else if (msCtime.isPresent()) {
lc.ctime = msCtime.get();
+ lc.storeSystemtimeAsLedgerCreationTime = false;
}
if (data.hasAckQuorumSize()) {
lc.ackQuorumSize = data.getAckQuorumSize();
@@ -583,7 +601,6 @@ boolean isConflictWith(LedgerMetadata newMeta) {
if (metadataFormatVersion != newMeta.metadataFormatVersion ||
ensembleSize != newMeta.ensembleSize ||
writeQuorumSize != newMeta.writeQuorumSize ||
- ctime != newMeta.ctime ||
ackQuorumSize != newMeta.ackQuorumSize ||
length != newMeta.length ||
state != newMeta.state ||
@@ -592,6 +609,14 @@ boolean isConflictWith(LedgerMetadata newMeta) {
!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;
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 5cb14686e..d7b913fee 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
@@ -33,11 +33,11 @@
*/
public class LedgerMetadataTest {
+ private static final byte[] passwd = "testPasswd".getBytes(UTF_8);
+
@Test
public void testStoreSystemtimeAsLedgerCtimeEnabled()
throws Exception {
- byte[] passwd = "testPasswd".getBytes(UTF_8);
-
LedgerMetadata lm = new LedgerMetadata(
3,
3,
@@ -53,8 +53,6 @@ public void testStoreSystemtimeAsLedgerCtimeEnabled()
@Test
public void testStoreSystemtimeAsLedgerCtimeDisabled()
throws Exception {
- byte[] passwd = "testPasswd".getBytes(UTF_8);
-
LedgerMetadata lm = new LedgerMetadata(
3,
3,
@@ -67,4 +65,60 @@ public void testStoreSystemtimeAsLedgerCtimeDisabled()
assertFalse(format.hasCtime());
}
+ @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));
+ }
+
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services