Skip to content

Commit 2bf77f0

Browse files
author
ZhangJian He
committed
improve: remove asserts from code
Signed-off-by: ZhangJian He <[email protected]>
1 parent 9f202b0 commit 2bf77f0

22 files changed

+148
-153
lines changed

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryMemTable.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ public EntryKeyValue put(EntryKey k, EntryKeyValue v) {
7575

7676
@Override
7777
public EntryKeyValue putIfAbsent(EntryKey k, EntryKeyValue v) {
78+
if (!k.equals(v)) {
79+
throw new IllegalArgumentException("Key and value should be same");
80+
}
7881
assert k.equals(v);
7982
return super.putIfAbsent(v, v);
8083
}
@@ -275,11 +278,15 @@ long flushSnapshot(final SkipListFlusher flusher, Checkpoint checkpoint) throws
275278
*/
276279
void clearSnapshot(final EntrySkipList keyValues) {
277280
// Caller makes sure that keyValues not empty
278-
assert !keyValues.isEmpty();
281+
if (keyValues.isEmpty()) {
282+
throw new IllegalStateException("Snapshot is empty");
283+
}
279284
this.lock.writeLock().lock();
280285
try {
286+
if (this.snapshot != keyValues) {
287+
throw new IllegalStateException("Snapshot mismatch");
288+
}
281289
// create a new snapshot and let the old one go.
282-
assert this.snapshot == keyValues;
283290
this.snapshot = EntrySkipList.EMPTY_VALUE;
284291
} finally {
285292
this.lock.writeLock().unlock();
@@ -371,7 +378,9 @@ private EntryKeyValue cloneWithAllocator(long ledgerId, long entryId, final Byte
371378
return newEntry(ledgerId, entryId, entry);
372379
}
373380

374-
assert alloc.getData() != null;
381+
if (alloc.getData() == null) {
382+
throw new IllegalStateException("MemorySlice.getData() should not be null");
383+
}
375384
entry.get(alloc.getData(), alloc.getOffset(), len);
376385
return new EntryKeyValue(ledgerId, entryId, alloc.getData(), alloc.getOffset(), len);
377386
}

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexInMemPageMgr.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,9 @@ void putEntryOffset(long ledger, long entry, long offset) throws IOException {
571571
LedgerEntryPage lep = null;
572572
try {
573573
lep = getLedgerEntryPage(ledger, pageEntry);
574-
assert lep != null;
574+
if (lep == null) {
575+
throw new Bookie.NoLedgerException(ledger);
576+
}
575577
lep.setOffset(offset, offsetInPage * LedgerEntryPage.getIndexEntrySize());
576578
} catch (FileInfo.FileInfoDeletedException e) {
577579
throw new Bookie.NoLedgerException(ledger);

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@
4040
public class LedgerDescriptorImpl extends LedgerDescriptor {
4141
private static final Logger LOG = LoggerFactory.getLogger(LedgerDescriptorImpl.class);
4242
final LedgerStorage ledgerStorage;
43-
private long ledgerId;
43+
private final long ledgerId;
4444
final byte[] masterKey;
4545

46-
private AtomicBoolean fenceEntryPersisted = new AtomicBoolean();
46+
private final AtomicBoolean fenceEntryPersisted = new AtomicBoolean();
4747
private CompletableFuture<Boolean> logFenceResult = null;
4848

4949
LedgerDescriptorImpl(byte[] masterKey,

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorReadOnlyImpl.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,16 @@ public class LedgerDescriptorReadOnlyImpl extends LedgerDescriptorImpl {
3434

3535
@Override
3636
boolean setFenced() throws IOException {
37-
assert false;
3837
throw new IOException("Invalid action on read only descriptor");
3938
}
4039

4140
@Override
4241
long addEntry(ByteBuf entry) throws IOException {
43-
assert false;
4442
throw new IOException("Invalid action on read only descriptor");
4543
}
4644

4745
@Override
4846
void checkAccess(byte[] masterKey) throws BookieException, IOException {
49-
assert false;
5047
throw new IOException("Invalid action on read only descriptor");
5148
}
5249
}

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SkipListArena.java

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
* </p>
4646
*/
4747
public class SkipListArena {
48-
private AtomicReference<Chunk> curChunk = new AtomicReference<Chunk>();
48+
private final AtomicReference<Chunk> curChunk = new AtomicReference<Chunk>();
4949

5050
final int chunkSize;
5151

@@ -54,7 +54,9 @@ public class SkipListArena {
5454
public SkipListArena(ServerConfiguration cfg) {
5555
chunkSize = cfg.getSkipListArenaChunkSize();
5656
maxAlloc = cfg.getSkipListArenaMaxAllocSize();
57-
assert maxAlloc <= chunkSize;
57+
if (maxAlloc > chunkSize) {
58+
throw new IllegalArgumentException("maxAlloc should be less than or equal to chunkSize");
59+
}
5860
}
5961

6062
/**
@@ -64,7 +66,9 @@ public SkipListArena(ServerConfiguration cfg) {
6466
* </p>
6567
*/
6668
public MemorySlice allocateBytes(int size) {
67-
assert size >= 0;
69+
if (size < 0) {
70+
throw new IllegalArgumentException("Invalid size: " + size);
71+
}
6872

6973
// Callers should satisfy large allocations directly from JVM since they
7074
// don't cause fragmentation as badly.
@@ -135,10 +139,10 @@ private static class Chunk {
135139
* Offset for the next allocation, or the sentinel value -1
136140
* which implies that the chunk is still uninitialized.
137141
*/
138-
private AtomicInteger nextFreeOffset = new AtomicInteger(UNINITIALIZED);
142+
private final AtomicInteger nextFreeOffset = new AtomicInteger(UNINITIALIZED);
139143

140144
/** Total number of allocations satisfied from this buffer. */
141-
private AtomicInteger allocCount = new AtomicInteger();
145+
private final AtomicInteger allocCount = new AtomicInteger();
142146

143147
/** Size of chunk in bytes. */
144148
private final int size;
@@ -158,17 +162,20 @@ private Chunk(int size) {
158162
* threads calling alloc(), who will block until the allocation is complete.
159163
*/
160164
public void init() {
161-
assert nextFreeOffset.get() == UNINITIALIZED;
165+
if (nextFreeOffset.get() != UNINITIALIZED) {
166+
throw new IllegalStateException("Double-init");
167+
}
162168
try {
163169
data = new byte[size];
164170
} catch (OutOfMemoryError e) {
165-
boolean failInit = nextFreeOffset.compareAndSet(UNINITIALIZED, OOM);
166-
assert failInit; // should be true.
171+
nextFreeOffset.compareAndSet(UNINITIALIZED, OOM);
167172
throw e;
168173
}
169174
// Mark that it's ready for use
170175
boolean okInit = nextFreeOffset.compareAndSet(UNINITIALIZED, 0);
171-
assert okInit; // single-threaded call
176+
if (!okInit) {
177+
throw new IllegalStateException("Double-init");
178+
}
172179
}
173180

174181
/**

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookieWatcherImpl.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,6 @@ public void initialBlockingBookieRead() throws BKException {
226226
CompletableFuture<?> readonly;
227227
synchronized (this) {
228228
if (initialReadonlyBookiesFuture == null) {
229-
assert initialWritableBookiesFuture == null;
230-
231229
writable = this.registrationClient.watchWritableBookies(
232230
bookies -> processWritableBookiesChanged(bookies.getValue()));
233231

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerFragmentReplicator.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,10 @@ private void replicateFragmentInternal(final LedgerHandle lh,
158158
* INVALID_ENTRY_ID and viceversa.
159159
*/
160160
if (startEntryId == INVALID_ENTRY_ID ^ endEntryId == INVALID_ENTRY_ID) {
161-
LOG.error("For LedgerFragment: {}, seeing inconsistent firstStoredEntryId: {} and lastStoredEntryId: {}",
161+
String msg = String.format("For LedgerFragment: %s, seeing inconsistent firstStoredEntryId: %s and lastStoredEntryId: %s",
162162
lf, startEntryId, endEntryId);
163-
assert false;
163+
LOG.error(msg);
164+
throw new IllegalStateException(msg);
164165
}
165166

166167
if (startEntryId > endEntryId || endEntryId <= INVALID_ENTRY_ID) {
@@ -231,13 +232,12 @@ private void replicateFragmentInternal(final LedgerHandle lh,
231232
void replicate(final LedgerHandle lh, final LedgerFragment lf,
232233
final AsyncCallback.VoidCallback ledgerFragmentMcb,
233234
final Set<BookieId> targetBookieAddresses,
234-
final BiConsumer<Long, Long> onReadEntryFailureCallback)
235-
throws InterruptedException {
236-
Set<LedgerFragment> partionedFragments = splitIntoSubFragments(lh, lf,
235+
final BiConsumer<Long, Long> onReadEntryFailureCallback) {
236+
Set<LedgerFragment> partitionedFragments = splitIntoSubFragments(lh, lf,
237237
bkc.getConf().getRereplicationEntryBatchSize());
238238
LOG.info("Replicating fragment {} in {} sub fragments.",
239-
lf, partionedFragments.size());
240-
replicateNextBatch(lh, partionedFragments.iterator(),
239+
lf, partitionedFragments.size());
240+
replicateNextBatch(lh, partitionedFragments.iterator(),
241241
ledgerFragmentMcb, targetBookieAddresses, onReadEntryFailureCallback);
242242
}
243243

@@ -300,9 +300,10 @@ static Set<LedgerFragment> splitIntoSubFragments(LedgerHandle lh,
300300
* INVALID_ENTRY_ID and viceversa.
301301
*/
302302
if (firstEntryId == INVALID_ENTRY_ID ^ lastEntryId == INVALID_ENTRY_ID) {
303-
LOG.error("For LedgerFragment: {}, seeing inconsistent firstStoredEntryId: {} and lastStoredEntryId: {}",
303+
String msg = String.format("For LedgerFragment: %s, seeing inconsistent firstStoredEntryId: %s and lastStoredEntryId: %s",
304304
ledgerFragment, firstEntryId, lastEntryId);
305-
assert false;
305+
LOG.error(msg);
306+
throw new IllegalStateException(msg);
306307
}
307308

308309
long numberOfEntriesToReplicate = firstEntryId == INVALID_ENTRY_ID ? 0 : (lastEntryId - firstEntryId) + 1;

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -460,11 +460,11 @@ public void remove() {
460460

461461
@Override
462462
public boolean hasNext() {
463-
if (curBatch.size() > 0) {
463+
if (!curBatch.isEmpty()) {
464464
return true;
465465
}
466466

467-
while (queue.size() > 0 && curBatch.size() == 0) {
467+
while (!queue.isEmpty() && curBatch.isEmpty()) {
468468
String parent = queue.remove();
469469
try {
470470
for (String c : zkc.getChildren(parent, false)) {
@@ -491,12 +491,14 @@ public boolean hasNext() {
491491
throw new RuntimeException("Error reading list", e);
492492
}
493493
}
494-
return curBatch.size() > 0;
494+
return !curBatch.isEmpty();
495495
}
496496

497497
@Override
498498
public UnderreplicatedLedger next() {
499-
assert curBatch.size() > 0;
499+
if (curBatch.isEmpty()) {
500+
throw new IllegalStateException("No more elements");
501+
}
500502
return curBatch.remove();
501503
}
502504
};

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/AuthHandler.java

Lines changed: 33 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,9 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
8989
super.channelRead(ctx, msg);
9090
} else if (msg instanceof BookieProtocol.AuthRequest) { // pre-PB-client
9191
BookieProtocol.AuthRequest req = (BookieProtocol.AuthRequest) msg;
92-
assert (req.getOpCode() == BookieProtocol.AUTH);
92+
if (BookieProtocol.AUTH != req.getOpCode()) {
93+
throw new IllegalStateException("Received message other than auth message");
94+
}
9395
if (checkAuthPlugin(req.getAuthMessage(), ctx.channel())) {
9496
byte[] payload = req
9597
.getAuthMessage()
@@ -270,45 +272,44 @@ public void channelInactive(ChannelHandlerContext ctx) throws Exception {
270272

271273
@Override
272274
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
273-
assert (authProvider != null);
275+
if (authProvider == null) {
276+
throw new IllegalStateException("Auth provider is not initialized");
277+
}
274278

275279
if (authenticated) {
276280
super.channelRead(ctx, msg);
277281
} else if (msg instanceof BookkeeperProtocol.Response) {
278282
BookkeeperProtocol.Response resp = (BookkeeperProtocol.Response) msg;
279-
if (null == resp.getHeader().getOperation()) {
280-
LOG.info("dropping received malformed message {} from bookie {}", msg, ctx.channel());
281-
// drop the message without header
282-
} else {
283-
switch (resp.getHeader().getOperation()) {
284-
case START_TLS:
285-
super.channelRead(ctx, msg);
286-
break;
287-
case AUTH:
288-
if (resp.getStatus() != BookkeeperProtocol.StatusCode.EOK) {
289-
authenticationError(ctx, resp.getStatus().getNumber());
290-
} else {
291-
assert (resp.hasAuthResponse());
292-
BookkeeperProtocol.AuthMessage am = resp.getAuthResponse();
293-
if (AUTHENTICATION_DISABLED_PLUGIN_NAME.equals(am.getAuthPluginName())){
294-
SocketAddress remote = ctx.channel().remoteAddress();
295-
LOG.info("Authentication is not enabled."
283+
switch (resp.getHeader().getOperation()) {
284+
case START_TLS:
285+
super.channelRead(ctx, msg);
286+
break;
287+
case AUTH:
288+
if (resp.getStatus() != BookkeeperProtocol.StatusCode.EOK) {
289+
authenticationError(ctx, resp.getStatus().getNumber());
290+
} else {
291+
if (!resp.hasAuthResponse()) {
292+
throw new IllegalStateException("Auth response is missing in the message");
293+
}
294+
AuthMessage am = resp.getAuthResponse();
295+
if (AUTHENTICATION_DISABLED_PLUGIN_NAME.equals(am.getAuthPluginName())) {
296+
SocketAddress remote = ctx.channel().remoteAddress();
297+
LOG.info("Authentication is not enabled."
296298
+ "Considering this client {} authenticated", remote);
297-
AuthHandshakeCompleteCallback cb = new AuthHandshakeCompleteCallback(ctx);
298-
cb.operationComplete(BKException.Code.OK, null);
299-
return;
300-
}
301-
byte[] payload = am.getPayload().toByteArray();
302-
authProvider.process(AuthToken.wrap(payload), new AuthRequestCallback(ctx,
303-
authProviderFactory.getPluginName()));
299+
AuthHandshakeCompleteCallback cb = new AuthHandshakeCompleteCallback(ctx);
300+
cb.operationComplete(BKException.Code.OK, null);
301+
return;
304302
}
305-
break;
306-
default:
307-
LOG.warn("dropping received message {} from bookie {}", msg, ctx.channel());
308-
// else just drop the message,
309-
// we're not authenticated so nothing should be coming through
310-
break;
303+
byte[] payload = am.getPayload().toByteArray();
304+
authProvider.process(AuthToken.wrap(payload), new AuthRequestCallback(ctx,
305+
authProviderFactory.getPluginName()));
311306
}
307+
break;
308+
default:
309+
LOG.warn("dropping received message {} from bookie {}", msg, ctx.channel());
310+
// else just drop the message,
311+
// we're not authenticated so nothing should be coming through
312+
break;
312313
}
313314
} else if (msg instanceof BookieProtocol.Response) {
314315
BookieProtocol.Response resp = (BookieProtocol.Response) msg;

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtocol.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,6 @@ boolean hasMasterKey() {
237237
}
238238

239239
byte[] getMasterKey() {
240-
assert hasMasterKey();
241240
return masterKey;
242241
}
243242

0 commit comments

Comments
 (0)