eolivelli commented on a change in pull request #2936:
URL: https://github.com/apache/bookkeeper/pull/2936#discussion_r772928202



##########
File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/MockLedgerStorage.java
##########
@@ -0,0 +1,307 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.bookkeeper.bookie;
+
+import com.google.common.util.concurrent.RateLimiter;
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.ByteBufAllocator;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.PrimitiveIterator;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.apache.bookkeeper.bookie.CheckpointSource.Checkpoint;
+import org.apache.bookkeeper.common.util.Watcher;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.meta.LedgerManager;
+import org.apache.bookkeeper.stats.StatsLogger;
+
+/**
+ * A mock for running tests that require ledger storage.
+ */
+public class MockLedgerStorage implements LedgerStorage {

Review comment:
       it would be great, as a follow up work, to move this class out of the 
"tests" package, this way people can use this LedgerStorage implementation in 
unit tests or downstream applications that need a Bookie.
   
   So this class + don't write to the journal = lightweight bookie for unit 
tests
   
   btw people can "depend" on bookkeeper-server tests, so not a big deal but we 
could add more visibility
   

##########
File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java
##########
@@ -340,14 +359,61 @@ public static LifecycleComponentStack 
buildBookieServer(BookieConfiguration conf
         LedgerDirsManager indexDirsManager = 
BookieResources.createIndexDirsManager(
                 conf.getServerConf(), diskChecker, 
bookieStats.scope(LD_INDEX_SCOPE), ledgerDirsManager);
 
-        CookieValidation cookieValidation = new 
LegacyCookieValidation(conf.getServerConf(), rm);
-        
cookieValidation.checkCookies(storageDirectoriesFromConf(conf.getServerConf()));
-
         ByteBufAllocatorWithOomHandler allocator = 
BookieResources.createAllocator(conf.getServerConf());
 
+        UncleanShutdownDetection uncleanShutdownDetection = new 
UncleanShutdownDetectionImpl(ledgerDirsManager);
+        if (uncleanShutdownDetection.lastShutdownWasUnclean()) {
+            log.info("Unclean shutdown detected. The bookie did not register a 
graceful shutdown prior to this boot.");
+        }
+
         // bookie takes ownership of storage, so shuts it down
-        LedgerStorage storage = BookieResources.createLedgerStorage(
-                conf.getServerConf(), ledgerManager, ledgerDirsManager, 
indexDirsManager, bookieStats, allocator);
+        LedgerStorage storage = null;
+        DataIntegrityCheck integCheck = null;
+
+        if (conf.getServerConf().isDataIntegrityCheckingEnabled()) {
+            StatsLogger clientStats = bookieStats.scope(CLIENT_SCOPE);
+            ClientConfiguration clientConfiguration = new 
ClientConfiguration(conf.getServerConf());
+            
clientConfiguration.setClientRole(ClientConfiguration.CLIENT_ROLE_SYSTEM);
+            BookKeeper bkc = 
BookKeeper.forConfig(clientConfiguration).statsLogger(clientStats).build();
+            serverBuilder.addComponent(new 
AutoCloseableLifecycleComponent("bkc", bkc));
+
+            BookieId bookieId = BookieImpl.getBookieId(conf.getServerConf());
+            ExecutorService rxExecutor = Executors.newFixedThreadPool(

Review comment:
       it looks like we are not shutting down this ExecutorService.
   not a big deal because we are in the Main class and the JVM will die, but if 
there is a clean way to shutdown this thread pool it would be better

##########
File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java
##########
@@ -327,6 +328,13 @@ public boolean ledgerExists(long ledgerId) throws 
IOException {
         return ledgerCache.ledgerExists(ledgerId);
     }
 
+    @Override
+    public boolean entryExists(long ledgerId, long entryId) throws IOException 
{
+        //Implementation should be as simple as what's below, but this needs 
testing
+        //return ledgerCache.getEntryOffset(ledgerId, entryId) > 0;
+        throw new UnsupportedOperationException("entry exists not supported");

Review comment:
       makes sense




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to