This is an automated email from the ASF dual-hosted git repository.
ivank 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 9b3c6b7 Make LedgerMetadata fields for closing immutable
9b3c6b7 is described below
commit 9b3c6b73d8f667d52eb78025f07d3aedec6e9de0
Author: Ivan Kelly <[email protected]>
AuthorDate: Fri Nov 23 14:32:31 2018 +0000
Make LedgerMetadata fields for closing immutable
Master issue: #281
Reviewers: Enrico Olivelli <[email protected]>, Sijie Guo
<[email protected]>
This closes #1831 from ivankelly/close-immut
---
.../apache/bookkeeper/client/LedgerMetadata.java | 39 ++++++++--------------
.../bookkeeper/client/LedgerMetadataBuilder.java | 11 ++----
.../bookkeeper/client/LedgerRecoveryTest.java | 6 ++--
3 files changed, 18 insertions(+), 38 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 bf5e9f5..4f67077 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
@@ -78,9 +78,9 @@ public class LedgerMetadata implements
org.apache.bookkeeper.client.api.LedgerMe
private final int writeQuorumSize;
private final int ackQuorumSize;
- private LedgerMetadataFormat.State state;
- private long length;
- private long lastEntryId;
+ private final LedgerMetadataFormat.State state;
+ private final long length;
+ private final long lastEntryId;
private final long ctime;
final boolean storeCtime; // non-private so builder can access for copy
@@ -107,18 +107,22 @@ public class LedgerMetadata implements
org.apache.bookkeeper.client.api.LedgerMe
boolean storeCtime,
Map<String, byte[]> customMetadata) {
checkArgument(ensembles.size() > 0, "There must be at least one
ensemble in the ledger");
+ if (state == LedgerMetadataFormat.State.CLOSED) {
+ checkArgument(length.isPresent(), "Closed ledger must have a
length");
+ checkArgument(lastEntryId.isPresent(), "Closed ledger must have a
last entry");
+ } else {
+ checkArgument(!length.isPresent(), "Non-closed ledger must not
have a length");
+ checkArgument(!lastEntryId.isPresent(), "Non-closed ledger must
not have a last entry");
+ }
this.metadataFormatVersion = metadataFormatVersion;
this.ensembleSize = ensembleSize;
this.writeQuorumSize = writeQuorumSize;
this.ackQuorumSize = ackQuorumSize;
this.state = state;
- if (lastEntryId.isPresent()) {
- this.lastEntryId = lastEntryId.get();
- } else {
- this.lastEntryId = LedgerHandle.INVALID_ENTRY_ID;
- }
- length.ifPresent((l) -> this.length = l);
+
+ this.lastEntryId = lastEntryId.orElse(LedgerHandle.INVALID_ENTRY_ID);
+ this.length = length.orElse(0L);
this.ensembles = Collections.unmodifiableNavigableMap(
ensembles.entrySet().stream().collect(TreeMap::new,
@@ -248,10 +252,6 @@ public class LedgerMetadata implements
org.apache.bookkeeper.client.api.LedgerMe
return length;
}
- void setLength(long length) {
- this.length = length;
- }
-
@Override
public boolean isClosed() {
return state == LedgerMetadataFormat.State.CLOSED;
@@ -265,19 +265,6 @@ public class LedgerMetadata implements
org.apache.bookkeeper.client.api.LedgerMe
return state;
}
- void setState(LedgerMetadataFormat.State state) {
- this.state = state;
- }
-
- void markLedgerInRecovery() {
- state = LedgerMetadataFormat.State.IN_RECOVERY;
- }
-
- void close(long entryId) {
- lastEntryId = entryId;
- state = LedgerMetadataFormat.State.CLOSED;
- }
-
List<BookieSocketAddress> getCurrentEnsemble() {
return currentEnsemble;
}
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadataBuilder.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadataBuilder.java
index 74ec717..4a42c70 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadataBuilder.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadataBuilder.java
@@ -73,14 +73,9 @@ public class LedgerMetadataBuilder {
builder.ackQuorumSize = other.getAckQuorumSize();
builder.state = other.getState();
-
- long lastEntryId = other.getLastEntryId();
- if (lastEntryId != LedgerHandle.INVALID_ENTRY_ID) {
- builder.lastEntryId = Optional.of(lastEntryId);
- }
- long length = other.getLength();
- if (length > 0) {
- builder.length = Optional.of(length);
+ if (builder.state == LedgerMetadataFormat.State.CLOSED) {
+ builder.lastEntryId = Optional.of(other.getLastEntryId());
+ builder.length = Optional.of(other.getLength());
}
builder.ensembles.putAll(other.getAllEnsembles());
diff --git
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerRecoveryTest.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerRecoveryTest.java
index 6a21da2..9a927b6 100644
---
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerRecoveryTest.java
+++
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerRecoveryTest.java
@@ -84,10 +84,8 @@ public class LedgerRecoveryTest extends
BookKeeperClusterTestCase {
/*
* Check if has recovered properly.
*/
- assertTrue("Has not recovered correctly: " +
afterlh.getLastAddConfirmed(),
- afterlh.getLastAddConfirmed() == numEntries - 1);
- assertTrue("Has not set the length correctly: " + afterlh.getLength()
+ ", " + length,
- afterlh.getLength() == length);
+ assertEquals("Has not recovered correctly", numEntries - 1,
afterlh.getLastAddConfirmed());
+ assertEquals("Has not set the length correctly", length,
afterlh.getLength());
}
@Test
