From d2ec418d6010332f55828d53c613a3c3e9d03d9e Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Thu, 05 Sep 2013 09:32:32 +0000
Subject: [PATCH] OPENDJ-1116 Introduce abstraction for the changelog DB

---
 opends/src/server/org/opends/server/replication/server/ReplicationServer.java                                         |   31 +++++-----
 opends/src/server/org/opends/server/replication/server/ECLServerHandler.java                                          |   13 +--
 opends/src/server/org/opends/server/replication/server/changelog/api/ChangeNumberIndexDB.java                         |    9 ---
 opends/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandler.java                             |   33 +----------
 opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandlerTest.java |   32 ++++------
 opends/src/messages/messages/replication.properties                                                                   |    8 ++
 6 files changed, 41 insertions(+), 85 deletions(-)

diff --git a/opends/src/messages/messages/replication.properties b/opends/src/messages/messages/replication.properties
index 222a912..cf409c6 100644
--- a/opends/src/messages/messages/replication.properties
+++ b/opends/src/messages/messages/replication.properties
@@ -310,8 +310,8 @@
  domain %s from server %s to all other servers of the topology is forbidden as \
  the source server has some fractional configuration : only fractional servers \
  in a replicated topology does not make sense
-MILD_ERR_DRAFT_CHANGENUMBER_DATABASE_173=An error occurred when accessing the \
- database of the draft change number : %s
+MILD_ERR_CHANGENUMBER_DATABASE_173=An error occurred when accessing the \
+ change number database : %s
 SEVERE_ERR_INITIALIZATION_FAILED_NOCONN_174=The initialization failed because \
  the domain %s is not connected to a replication server
 SEVERE_ERR_FRACTIONAL_COULD_NOT_RETRIEVE_CONFIG_175=Could not retrieve the \
@@ -492,3 +492,7 @@
 SEVERE_ERR_REPLICATIONDB_CANNOT_PROCESS_CHANGE_RECORD_215=Replication server RS(%d) \
  failed to parse change record with changenumber %s from the database. Error: %s
 SEVERE_ERR_SESSION_STARTUP_INTERRUPTED_216=%s was interrupted in the startup phase
+MILD_ERR_READING_FIRST_THEN_LAST_IN_CHANGENUMBER_DATABASE_217=An error occurred \
+ when accessing the change number database: impossible to read the last record \
+ after having successfully read the first. Database might have been cleaned or \
+ closed between successive reads.
diff --git a/opends/src/server/org/opends/server/replication/server/ECLServerHandler.java b/opends/src/server/org/opends/server/replication/server/ECLServerHandler.java
index 364b956..e3cdc07 100644
--- a/opends/src/server/org/opends/server/replication/server/ECLServerHandler.java
+++ b/opends/src/server/org/opends/server/replication/server/ECLServerHandler.java
@@ -571,16 +571,13 @@
     {
       // Request filter DOES NOT contain any first change number
       // So we'll generate from the first change number in the DraftCNdb
-      if (cnIndexDB.isEmpty())
-      {
-        // FIXME JNR if we find a way to make draftCNDb.isEmpty() a non costly
-        // operation, then I think we can move this check to the top of this
-        // method
+      final CNIndexData firstCNData = cnIndexDB.getFirstCNIndexData();
+      if (firstCNData == null)
+      { // DB is empty or closed
         isEndOfCNIndexDBReached = true;
         return null;
       }
 
-      final CNIndexData firstCNData = cnIndexDB.getFirstCNIndexData();
       final long firstChangeNumber = firstCNData.getChangeNumber();
       final String crossDomainStartState = firstCNData.getPreviousCookie();
       cnIndexDBCursor = cnIndexDB.getCursorFrom(firstChangeNumber);
@@ -630,13 +627,13 @@
     {
       // startChangeNumber is between first and potential last and has never
       // been returned yet
-      if (cnIndexDB.isEmpty())
+      final CNIndexData lastCNData = cnIndexDB.getLastCNIndexData();
+      if (lastCNData == null)
       {
         isEndOfCNIndexDBReached = true;
         return null;
       }
 
-      final CNIndexData lastCNData = cnIndexDB.getLastCNIndexData();
       final long lastKey = lastCNData.getChangeNumber();
       final String crossDomainStartState = lastCNData.getPreviousCookie();
       cnIndexDBCursor = cnIndexDB.getCursorFrom(lastKey);
diff --git a/opends/src/server/org/opends/server/replication/server/ReplicationServer.java b/opends/src/server/org/opends/server/replication/server/ReplicationServer.java
index 11f6507..7ea509d 100644
--- a/opends/src/server/org/opends/server/replication/server/ReplicationServer.java
+++ b/opends/src/server/org/opends/server/replication/server/ReplicationServer.java
@@ -914,18 +914,6 @@
             TRACER.debugCaught(DebugLogLevel.WARNING, ignored);
           }
         }
-
-        try
-        {
-          lastGeneratedChangeNumber = cnIndexDB.getLastChangeNumber();
-        }
-        catch (Exception ignored)
-        {
-          if (debugEnabled())
-          {
-            TRACER.debugCaught(DebugLogLevel.WARNING, ignored);
-          }
-        }
       }
     }
   }
@@ -1638,16 +1626,19 @@
         if (cnIndexDB == null)
         {
           cnIndexDB = new DraftCNDbHandler(this, this.dbEnv);
-          lastGeneratedChangeNumber = cnIndexDB.getLastChangeNumber();
+          final CNIndexData lastCNData = cnIndexDB.getLastCNIndexData();
+          // initialization of the lastGeneratedChangeNumebr from the DB content
+          // if DB is empty => lastCNData is null => Default to 0
+          lastGeneratedChangeNumber =
+              (lastCNData != null) ? lastCNData.getChangeNumber() : 0;
         }
         return cnIndexDB;
       }
       catch (Exception e)
       {
         TRACER.debugCaught(DebugLogLevel.ERROR, e);
-        MessageBuilder mb = new MessageBuilder();
-        mb.append(ERR_DRAFT_CHANGENUMBER_DATABASE.get(""));
-        throw new DirectoryException(OPERATIONS_ERROR, mb.toMessage(), e);
+        Message message = ERR_CHANGENUMBER_DATABASE.get(e.getMessage());
+        throw new DirectoryException(OPERATIONS_ERROR, message, e);
       }
     }
   }
@@ -1715,6 +1706,14 @@
       String domainForLastCN = null;
       if (firstCNData != null)
       {
+        if (lastCNData == null)
+        {
+          // Edge case: DB was cleaned or closed in between call to getFirst*()
+          // and getLast*(). The only remaining solution is to fail fast.
+          throw new ChangelogException(
+              ERR_READING_FIRST_THEN_LAST_IN_CHANGENUMBER_DATABASE.get());
+        }
+
         dbEmpty = false;
         firstChangeNumber = firstCNData.getChangeNumber();
         lastChangeNumber = lastCNData.getChangeNumber();
diff --git a/opends/src/server/org/opends/server/replication/server/changelog/api/ChangeNumberIndexDB.java b/opends/src/server/org/opends/server/replication/server/changelog/api/ChangeNumberIndexDB.java
index ab5abb1..5caee34 100644
--- a/opends/src/server/org/opends/server/replication/server/changelog/api/ChangeNumberIndexDB.java
+++ b/opends/src/server/org/opends/server/replication/server/changelog/api/ChangeNumberIndexDB.java
@@ -72,15 +72,6 @@
   CNIndexData getLastCNIndexData() throws ChangelogException;
 
   /**
-   * Get the last change number stored in this DB.
-   *
-   * @return Returns the last change number in this DB
-   * @throws ChangelogException
-   *           if a database problem occurs.
-   */
-  long getLastChangeNumber() throws ChangelogException;
-
-  /**
    * Add an update to the list of messages that must be saved to this DB managed
    * by this DB.
    * <p>
diff --git a/opends/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandler.java b/opends/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandler.java
index 5be5a8d..4a10839 100644
--- a/opends/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandler.java
+++ b/opends/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandler.java
@@ -169,18 +169,6 @@
     return db.readLastCNIndexData();
   }
 
-  /** {@inheritDoc} */
-  @Override
-  public long getLastChangeNumber() throws ChangelogException
-  {
-    final CNIndexData data = getLastCNIndexData();
-    if (data != null)
-    {
-      return data.getChangeNumber();
-    }
-    return 0;
-  }
-
   /**
    * Get the number of changes.
    * @return Returns the number of changes.
@@ -190,26 +178,11 @@
     return db.count();
   }
 
-  /**
-   * {@inheritDoc}
-   * <p>
-   * FIXME Find a way to implement this method in a more efficient manner.
-   * {@link com.sleepycat.je.Database#count()} javadoc mentions:
-   * <blockquote>Note that this method does scan a significant portion of the
-   * database and should be considered a fairly expensive
-   * operation.</blockquote>
-   * <p>
-   * It could be faster to:
-   * <ul>
-   * <li>open a cursor, check if the next entry exits, then close the cursor
-   * </li>
-   * <li>call <code>db.readFirstChangeNumber() != 0</code></li>
-   * </ul>
-   */
+  /** {@inheritDoc} */
   @Override
-  public boolean isEmpty()
+  public boolean isEmpty() throws ChangelogException
   {
-    return count() == 0;
+    return getLastCNIndexData() == null;
   }
 
   /**
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandlerTest.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandlerTest.java
index 7671929..b6e34f2 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandlerTest.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/changelog/je/DraftCNDbHandlerTest.java
@@ -37,7 +37,6 @@
 import org.opends.server.replication.server.ReplServerFakeConfiguration;
 import org.opends.server.replication.server.ReplicationServer;
 import org.opends.server.replication.server.changelog.api.CNIndexData;
-import org.opends.server.replication.server.changelog.api.ChangeNumberIndexDB;
 import org.opends.server.replication.server.changelog.api.ChangeNumberIndexDBCursor;
 import org.opends.server.replication.server.changelog.api.ChangelogException;
 import org.opends.server.replication.server.changelog.je.DraftCNDB.DraftCNDBCursor;
@@ -108,9 +107,9 @@
       handler.add(new CNIndexData(cn3, value3, baseDN3, csn3));
 
       // The ChangeNumber should not get purged
-      final long firstChangeNumber = getFirstChangeNumber(handler);
+      final long firstChangeNumber = handler.getFirstCNIndexData().getChangeNumber();
       assertEquals(firstChangeNumber, cn1);
-      assertEquals(getLastChangeNumber(handler), cn3);
+      assertEquals(handler.getLastCNIndexData().getChangeNumber(), cn3);
 
       DraftCNDBCursor dbc = handler.getReadCursor(firstChangeNumber);
       try
@@ -134,12 +133,13 @@
       handler.setPurgeDelay(100);
 
       // Check the db is cleared.
-      while (handler.count() != 0)
+      while (!handler.isEmpty())
       {
         Thread.sleep(200);
       }
-      assertEquals(getFirstChangeNumber(handler), 0);
-      assertEquals(getLastChangeNumber(handler), 0);
+      assertNull(handler.getFirstCNIndexData());
+      assertNull(handler.getLastCNIndexData());
+      assertEquals(handler.count(), 0);
     }
     finally
     {
@@ -235,10 +235,11 @@
       Thread.sleep(500);
 
       // Checks
-      assertEquals(getFirstChangeNumber(handler), cn1);
-      assertEquals(getLastChangeNumber(handler), cn3);
+      assertEquals(handler.getFirstCNIndexData().getChangeNumber(), cn1);
+      assertEquals(handler.getLastCNIndexData().getChangeNumber(), cn3);
 
       assertEquals(handler.count(), 3, "Db count");
+      assertFalse(handler.isEmpty());
 
       assertEquals(getPreviousCookie(handler, cn1), value1);
       assertEquals(getPreviousCookie(handler, cn2), value2);
@@ -256,9 +257,10 @@
       handler.clear();
 
       // Check the db is cleared.
-      assertEquals(getFirstChangeNumber(handler), 0);
-      assertEquals(getLastChangeNumber(handler), 0);
+      assertNull(handler.getFirstCNIndexData());
+      assertNull(handler.getLastCNIndexData());
       assertEquals(handler.count(), 0);
+      assertTrue(handler.isEmpty());
     }
     finally
     {
@@ -272,16 +274,6 @@
     }
   }
 
-  private long getFirstChangeNumber(ChangeNumberIndexDB handler) throws Exception
-  {
-    return handler.getFirstCNIndexData().getChangeNumber();
-  }
-
-  private long getLastChangeNumber(ChangeNumberIndexDB handler) throws Exception
-  {
-    return handler.getLastCNIndexData().getChangeNumber();
-  }
-
   private String getPreviousCookie(DraftCNDbHandler handler, long changeNumber) throws Exception
   {
     ChangeNumberIndexDBCursor cursor = handler.getCursorFrom(changeNumber);

--
Gitblit v1.10.0