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

[eolivelli]

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 <cguttapa...@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 <cguttapa...@salesforce.com>
    
    Reviewers: Enrico Olivelli <eolive...@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.");
     }
 }