[GitHub] [bookkeeper] dlg99 commented on a diff in pull request #3197: BP-47 (task3): Abstract interface for entrylogger

[GitBox <gi...@apache.org>]

dlg99 commented on code in PR #3197:
URL: https://github.com/apache/bookkeeper/pull/3197#discussion_r860226026


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java:
##########
@@ -180,7 +181,7 @@ public void initializeWithEntryLogger(ServerConfiguration conf,
                 LedgerManager ledgerManager,
                 LedgerDirsManager ledgerDirsManager,
                 LedgerDirsManager indexDirsManager,
-                EntryLogger entryLogger,
+                DefaultEntryLogger entryLogger,

Review Comment:
   ```suggestion
                   EntryLogger entryLogger,
   ```



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/bookie/ListActiveLedgersCommand.java:
##########
@@ -133,7 +133,7 @@ public void handler(ServerConfiguration bkConf, ActiveLedgerFlags cmdFlags)
             BKException.Code.OK, BKException.Code.ReadException);
           if (done.await(cmdFlags.timeout, TimeUnit.MILLISECONDS)){
             if  (resultCode.get() == BKException.Code.OK) {
-              EntryLogger entryLogger = new ReadOnlyEntryLogger(bkConf);
+              DefaultEntryLogger entryLogger = new ReadOnlyDefaultEntryLogger(bkConf);

Review Comment:
   ```suggestion
                 EntryLogger entryLogger = new ReadOnlyDefaultEntryLogger(bkConf);
   ```



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java:
##########
@@ -88,7 +89,7 @@
 public class InterleavedLedgerStorage implements CompactableLedgerStorage, EntryLogListener {
     private static final Logger LOG = LoggerFactory.getLogger(InterleavedLedgerStorage.class);
 
-    EntryLogger entryLogger;
+    DefaultEntryLogger entryLogger;

Review Comment:
   ```suggestion
       EntryLogger entryLogger;
   ```



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgersIndexRebuildOp.java:
##########
@@ -136,7 +136,7 @@ public boolean initiate()  {
     }
 
     private void scanEntryLogFiles(Set<Long> ledgers) throws IOException {
-        EntryLogger entryLogger = new EntryLogger(conf, new LedgerDirsManager(conf, conf.getLedgerDirs(),
+        DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, new LedgerDirsManager(conf, conf.getLedgerDirs(),

Review Comment:
   ```suggestion
           EntryLogger entryLogger = new DefaultEntryLogger(conf, new LedgerDirsManager(conf, conf.getLedgerDirs(),
   ```



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java:
##########
@@ -176,7 +176,7 @@ public class BookieShell implements Tool {
     File[] ledgerDirectories;
     File[] journalDirectories;
 
-    EntryLogger entryLogger = null;
+    DefaultEntryLogger entryLogger = null;

Review Comment:
   ```suggestion
       EntryLogger entryLogger = null;
   ```



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedStorageRegenerateIndexOp.java:
##########
@@ -94,7 +94,7 @@ public void initiate(boolean dryRun) throws IOException {
                 conf, diskChecker, NullStatsLogger.INSTANCE);
         LedgerDirsManager indexDirsManager = BookieResources.createIndexDirsManager(
                 conf, diskChecker,  NullStatsLogger.INSTANCE, ledgerDirsManager);
-        EntryLogger entryLogger = new EntryLogger(conf, ledgerDirsManager);
+        DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager);

Review Comment:
   ```suggestion
           EntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager);
   ```



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LocationsIndexRebuildOp.java:
##########
@@ -69,7 +69,7 @@ public void initiate() throws IOException {
 
         long startTime = System.nanoTime();
 
-        EntryLogger entryLogger = new EntryLogger(conf, new LedgerDirsManager(conf, conf.getLedgerDirs(),
+        DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, new LedgerDirsManager(conf, conf.getLedgerDirs(),

Review Comment:
   ```suggestion
           EntryLogger entryLogger = new DefaultEntryLogger(conf, new LedgerDirsManager(conf, conf.getLedgerDirs(),
   ```



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java:
##########
@@ -72,8 +78,8 @@
  * the actual ledger entry. The entry log files created by this class are
  * identified by a long.
  */
-public class EntryLogger {
-    private static final Logger LOG = LoggerFactory.getLogger(EntryLogger.class);
+public class DefaultEntryLogger implements EntryLogger {
+    private static final Logger LOG = LoggerFactory.getLogger(DefaultEntryLogger.class);
     static final long UNASSIGNED_LEDGERID = -1L;
     // log file suffix
     static final String LOG_FILE_SUFFIX = ".log";

Review Comment:
   I think these should be kept in the `EntryLogger` interface because these are not implementation-specific (UNASSIGNED_LEDGERID - definitely, LOG_FILE_SUFFIX - discussable)



-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org