This is an automated email from the ASF dual-hosted git repository.
zhaijia pushed a commit to branch branch-4.6
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/branch-4.6 by this push:
new 970363c ISSUE #726: Unit Tests failure with BKMetadataVersionException
970363c is described below
commit 970363c020ae58a3c07c70d53169fbc4e9057223
Author: Sijie Guo <[email protected]>
AuthorDate: Thu Nov 16 20:54:42 2017 +0800
ISSUE #726: Unit Tests failure with BKMetadataVersionException
Descriptions of the changes in this PR:
Problem:
The problem is introduced after making storing ctime optional. the
verification become inconsistent because the way how ctime was assigned and
compared. It causes the test cases failing in #726
Solution:
This change is to change `isConflict` to ignore comparing ctime if client
disables storing system time as ctime.
Author: Sijie Guo <[email protected]>
Reviewers: Enrico Olivelli <[email protected]>, Jia Zhai <None>
This closes #730 from sijie/fix_ctime_issue, closes #726
(cherry picked from commit c9a150b58fb1c8636dabbd7930bc3acffbf89881)
Signed-off-by: Jia Zhai <[email protected]>
---
.../apache/bookkeeper/client/LedgerMetadata.java | 35 ++++++++++--
.../bookkeeper/client/LedgerMetadataTest.java | 62 ++++++++++++++++++++--
2 files changed, 88 insertions(+), 9 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 d757542..44c8ce7 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 class LedgerMetadata {
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 class LedgerMetadata {
}
}
+ /**
+ * 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 class LedgerMetadata {
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 class LedgerMetadata {
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 class LedgerMetadata {
return writeQuorumSize;
}
+ public int getAckQuorumSize() {
+ return ackQuorumSize;
+ }
+
/**
* Get the creation timestamp of the ledger
* @return
@@ -186,8 +201,9 @@ public class LedgerMetadata {
return ctime;
}
- public int getAckQuorumSize() {
- return ackQuorumSize;
+ @VisibleForTesting
+ void setCtime(long ctime) {
+ this.ctime = ctime;
}
/**
@@ -430,8 +446,10 @@ public class LedgerMetadata {
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 @@ public class LedgerMetadata {
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 @@ public class LedgerMetadata {
!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 5cb1468..d7b913f 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 @@ import org.junit.Test;
*/
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 class LedgerMetadataTest {
@Test
public void testStoreSystemtimeAsLedgerCtimeDisabled()
throws Exception {
- byte[] passwd = "testPasswd".getBytes(UTF_8);
-
LedgerMetadata lm = new LedgerMetadata(
3,
3,
@@ -67,4 +65,60 @@ public class LedgerMetadataTest {
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));
+ }
+
}
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
