[bookkeeper] branch master updated: ISSUE #1541: Fix ReadOnlyBookie constructor

[eo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

eolivelli 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 4d5353b  ISSUE #1541: Fix ReadOnlyBookie constructor
4d5353b is described below

commit 4d5353bb7ea6c7f6cf2cb62992d122b265687b05
Author: cguttapalem <cg...@salesforce.com>
AuthorDate: Mon Jul 23 16:35:02 2018 +0200

    ISSUE #1541: Fix ReadOnlyBookie constructor
    
    Descriptions of the changes in this PR:
    
    ### Motivation
    
    ReadOnlyBookie doesn't sets shutdownHandler to its statemanager.
    
    There are couple of issues with how 'stateManager' is handled in ReadOnlyBookie,
    
    - ReadOnlyBookie creates its own stateManager in its constructor but it does not sets shutdownHandler.
    - Also, it is not correct to create stateManager instance in Bookie constructor and then again in ReadOnlyBookie constructor and override the instance created in super constructor.
    
    ### Changes
    
    - move initializeStateManager logic to a method and override it in ReadOnlyBookie.
    
    Master Issue: #1541
    
    Author: cguttapalem <cg...@salesforce.com>
    
    Reviewers: Enrico Olivelli <eo...@gmail.com>, Jia Zhai <None>, Sijie Guo <si...@apache.org>
    
    This closes #1557 from reddycharan/readonlybookiefix, closes #1541
---
 .../java/org/apache/bookkeeper/bookie/Bookie.java  |  8 ++++++--
 .../apache/bookkeeper/bookie/ReadOnlyBookie.java   | 22 +++++++++++++---------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
index 2b89387..79153dc 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
@@ -138,7 +138,7 @@ public class Bookie extends BookieCriticalThread {
     protected StateManager stateManager;
 
     // Expose Stats
-    private final StatsLogger statsLogger;
+    final StatsLogger statsLogger;
     private final Counter writeBytes;
     private final Counter readBytes;
     private final Counter forceLedgerOps;
@@ -641,7 +641,7 @@ public class Bookie extends BookieCriticalThread {
         } catch (MetadataException e) {
             throw new MetadataStoreException("Failed to initialize ledger manager", e);
         }
-        stateManager = new BookieStateManager(conf, statsLogger, metadataDriver, ledgerDirsManager);
+        stateManager = initializeStateManager();
         // register shutdown handler using trigger mode
         stateManager.setShutdownHandler(exitCode -> triggerBookieShutdown(exitCode));
         // Initialise ledgerDirMonitor. This would look through all the
@@ -745,6 +745,10 @@ public class Bookie extends BookieCriticalThread {
         readBytesStats = statsLogger.getOpStatsLogger(BOOKIE_READ_ENTRY_BYTES);
     }
 
+    StateManager initializeStateManager() throws IOException {
+        return new BookieStateManager(conf, statsLogger, metadataDriver, ledgerDirsManager);
+    }
+
     void readJournal() throws IOException, BookieException {
         long startTs = MathUtils.now();
         JournalScanner scanner = new JournalScanner() {
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ReadOnlyBookie.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ReadOnlyBookie.java
index 4bd3664..5125c07 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ReadOnlyBookie.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ReadOnlyBookie.java
@@ -41,7 +41,19 @@ public class ReadOnlyBookie extends Bookie {
     public ReadOnlyBookie(ServerConfiguration conf, StatsLogger statsLogger)
             throws IOException, KeeperException, InterruptedException, BookieException {
         super(conf, statsLogger);
-        stateManager = new BookieStateManager(conf, statsLogger, metadataDriver, getLedgerDirsManager()) {
+        if (conf.isReadOnlyModeEnabled()) {
+            stateManager.forceToReadOnly();
+        } else {
+            String err = "Try to init ReadOnly Bookie, while ReadOnly mode is not enabled";
+            LOG.error(err);
+            throw new IOException(err);
+        }
+        LOG.info("Running bookie in force readonly mode.");
+    }
+
+    @Override
+    StateManager initializeStateManager() throws IOException {
+        return new BookieStateManager(conf, statsLogger, metadataDriver, getLedgerDirsManager()) {
 
             @Override
             public void doTransitionToWritableMode() {
@@ -55,13 +67,5 @@ public class ReadOnlyBookie extends Bookie {
                 LOG.info("Skip transition to readonly mode for readonly bookie");
             }
         };
-        if (conf.isReadOnlyModeEnabled()) {
-            stateManager.forceToReadOnly();
-        } else {
-            String err = "Try to init ReadOnly Bookie, while ReadOnly mode is not enabled";
-            LOG.error(err);
-            throw new IOException(err);
-        }
-        LOG.info("Running bookie in force readonly mode.");
     }
 }