This is an automated email from the ASF dual-hosted git repository.
reddycharan 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 41b39c6 ISSUE #1967: make ledger creation and removal robust to zk
connectionloss
41b39c6 is described below
commit 41b39c6eff249729a698838e880ad77cc0c18c67
Author: Charan Reddy Guttapalem <[email protected]>
AuthorDate: Thu May 2 11:42:49 2019 -0700
ISSUE #1967: make ledger creation and removal robust to zk connectionloss
- fix test failures in TestLedgerMetadataSerDe
Reviewers: Enrico Olivelli <[email protected]>, Sijie Guo
<[email protected]>
This closes #2070 from reddycharan/fixzktestfailures, closes #1967
---
.../bookkeeper/meta/LedgerMetadataSerDe.java | 22 +++++++++++++++++-----
.../meta/AbstractZkLedgerManagerTest.java | 14 ++++++++++++--
.../bookkeeper/meta/TestLedgerMetadataSerDe.java | 13 ++++++-------
3 files changed, 35 insertions(+), 14 deletions(-)
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerMetadataSerDe.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerMetadataSerDe.java
index b34b34b..2ee441a 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerMetadataSerDe.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerMetadataSerDe.java
@@ -181,7 +181,12 @@ public class LedgerMetadataSerDe {
break;
}
- builder.setCtime(metadata.getCtime());
+ /** Hack to get around fact that ctime was never versioned
correctly */
+ if (LedgerMetadataUtils.shouldStoreCtime(metadata)) {
+ builder.setCtime(metadata.getCtime());
+ }
+
+
builder.setDigestType(apiToProtoDigestType(metadata.getDigestType()));
serializePassword(metadata.getPassword(), builder);
@@ -351,7 +356,7 @@ public class LedgerMetadataSerDe {
switch (metadataFormatVersion) {
case METADATA_FORMAT_VERSION_3:
- return parseVersion3Config(is);
+ return parseVersion3Config(is, metadataStoreCtime);
case METADATA_FORMAT_VERSION_2:
return parseVersion2Config(is, metadataStoreCtime);
case METADATA_FORMAT_VERSION_1:
@@ -365,12 +370,19 @@ public class LedgerMetadataSerDe {
}
}
- private static LedgerMetadata parseVersion3Config(InputStream is) throws
IOException {
+ private static LedgerMetadata parseVersion3Config(InputStream is,
Optional<Long> metadataStoreCtime)
+ throws IOException {
LedgerMetadataBuilder builder = LedgerMetadataBuilder.create()
- .withMetadataFormatVersion(METADATA_FORMAT_VERSION_3);
+ .withMetadataFormatVersion(METADATA_FORMAT_VERSION_3);
LedgerMetadataFormat.Builder formatBuilder =
LedgerMetadataFormat.newBuilder();
formatBuilder.mergeDelimitedFrom(is);
- decodeFormat(formatBuilder.build(), builder);
+ LedgerMetadataFormat data = formatBuilder.build();
+ decodeFormat(data, builder);
+ if (data.hasCtime()) {
+ builder.storingCreationTime(true);
+ } else if (metadataStoreCtime.isPresent()) {
+
builder.withCreationTime(metadataStoreCtime.get()).storingCreationTime(false);
+ }
return builder.build();
}
diff --git
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerTest.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerTest.java
index 0dc2b53..b54b7d9 100644
---
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerTest.java
+++
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerTest.java
@@ -124,7 +124,6 @@ public class AbstractZkLedgerManagerTest extends
MockZooKeeperTestCase {
this.metadata = LedgerMetadataBuilder.create()
.withDigestType(DigestType.CRC32C).withPassword(new byte[0])
.withEnsembleSize(5)
- .withMetadataFormatVersion(2)
.withWriteQuorumSize(3)
.withAckQuorumSize(3)
.newEnsembleEntry(0L, ensemble)
@@ -180,7 +179,18 @@ public class AbstractZkLedgerManagerTest extends
MockZooKeeperTestCase {
mockZkUtilsAsyncCreateFullPathOptimistic(
ledgerStr, CreateMode.PERSISTENT,
KeeperException.Code.NODEEXISTS.intValue(), null);
-
+ Stat stat = mock(Stat.class);
+ when(stat.getVersion()).thenReturn(1234);
+ when(stat.getCtime()).thenReturn(metadata.getCtime());
+ /*
+ * this is needed because in AbstractZkLedgerManager.readLedgerMetadata
+ * if MetadataFormatVersion is >2, then for createLedgerMetadata if we
+ * get NODEEXISTS exception then it will try to read to make sure
ledger
+ * creation is robust to ZK connection loss. Please check Issue #1967.
+ */
+ mockZkGetData(
+ ledgerStr, false,
+ KeeperException.Code.OK.intValue(), serDe.serialize(metadata),
stat);
try {
result(ledgerManager.createLedgerMetadata(ledgerId, metadata));
fail("Should fail to create ledger metadata if the ledger already
exists");
diff --git
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/TestLedgerMetadataSerDe.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/TestLedgerMetadataSerDe.java
index 1c9de74..a159e6c 100644
---
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/TestLedgerMetadataSerDe.java
+++
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/TestLedgerMetadataSerDe.java
@@ -122,9 +122,9 @@ public class TestLedgerMetadataSerDe {
}
@Test
- public void testPeggedToV2SerDe() throws Exception {
+ public void testPeggedToV3SerDe() throws Exception {
LedgerMetadataSerDe serDe = new LedgerMetadataSerDe();
- LedgerMetadata metadata =
LedgerMetadataBuilder.create().withMetadataFormatVersion(2)
+ LedgerMetadata metadata = LedgerMetadataBuilder.create()
.withEnsembleSize(3).withWriteQuorumSize(2).withAckQuorumSize(1)
.withPassword("foobar".getBytes(UTF_8)).withDigestType(DigestType.CRC32C)
.newEnsembleEntry(0L, Lists.newArrayList(new
BookieSocketAddress("192.0.2.1", 3181),
@@ -134,11 +134,11 @@ public class TestLedgerMetadataSerDe {
byte[] encoded = serDe.serialize(metadata);
LedgerMetadata decoded = serDe.parseConfig(encoded, Optional.empty());
- Assert.assertEquals(2, decoded.getMetadataFormatVersion());
+ Assert.assertEquals(LedgerMetadataSerDe.METADATA_FORMAT_VERSION_3,
decoded.getMetadataFormatVersion());
}
@Test
- public void testStoreSystemtimeAsLedgerCtimeEnabledWithVersion2()
+ public void testStoreSystemtimeAsLedgerCtimeEnabledWithNewerVersion()
throws Exception {
LedgerMetadata lm = LedgerMetadataBuilder.create()
.withEnsembleSize(3).withWriteQuorumSize(2).withAckQuorumSize(1)
@@ -161,9 +161,9 @@ public class TestLedgerMetadataSerDe {
}
@Test
- public void testStoreSystemtimeAsLedgerCtimeDisabledWithVersion2()
+ public void testStoreSystemtimeAsLedgerCtimeDisabledWithNewerVersion()
throws Exception {
- LedgerMetadata lm =
LedgerMetadataBuilder.create().withMetadataFormatVersion(2)
+ LedgerMetadata lm = LedgerMetadataBuilder.create()
.withEnsembleSize(3).withWriteQuorumSize(2).withAckQuorumSize(1)
.withPassword("foobar".getBytes(UTF_8)).withDigestType(DigestType.CRC32C)
.newEnsembleEntry(0L, Lists.newArrayList(
@@ -171,7 +171,6 @@ public class TestLedgerMetadataSerDe {
new BookieSocketAddress("192.0.2.2",
1234),
new BookieSocketAddress("192.0.2.3",
1234)))
.build();
-
LedgerMetadataSerDe serDe = new LedgerMetadataSerDe();
byte[] serialized = serDe.serialize(lm);
LedgerMetadata deserialized = serDe.parseConfig(serialized,
Optional.of(654321L));
