[jboss-cvs] JBossAS SVN: r57835 - in branches/JBoss_4_0_4_GA_CP/server/src: main/org/jboss/ejb/plugins/keygenerator/hilo resources/uuid-key-generator/META-INF

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Wed Oct 25 17:43:14 EDT 2006


Author: fnasser at redhat.com
Date: 2006-10-25 17:43:10 -0400 (Wed, 25 Oct 2006)
New Revision: 57835

Modified:
   branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGenerator.java
   branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGeneratorFactory.java
   branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGeneratorFactoryMBean.java
   branches/JBoss_4_0_4_GA_CP/server/src/resources/uuid-key-generator/META-INF/jboss-service.xml
Log:
ASPATCH-61: JBAS-3335: JBAS-3328 HiLoKeyGenerator.generateKey is not thread-safe

Modified: branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGenerator.java
===================================================================
--- branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGenerator.java	2006-10-25 21:23:32 UTC (rev 57834)
+++ branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGenerator.java	2006-10-25 21:43:10 UTC (rev 57835)
@@ -20,7 +20,6 @@
 * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
 */
 package org.jboss.ejb.plugins.keygenerator.hilo;
-
 import org.jboss.ejb.plugins.keygenerator.KeyGenerator;
 import org.jboss.ejb.plugins.cmp.jdbc.JDBCUtil;
 import org.jboss.logging.Logger;
@@ -32,7 +31,7 @@
 import java.sql.PreparedStatement;
 import java.sql.Connection;
 import java.sql.SQLException;
-
+import java.sql.ResultSet;
 /**
  * @author <a href="mailto:alex at jboss.org">Alexey Loubyansky</a>
  * @version <tt>$Revision$</tt>
@@ -42,6 +41,16 @@
 {
    private static long highestHi = 0;
 
+   public static synchronized long getHighestHi()
+   {
+      return highestHi;
+   }
+
+   public static synchronized void setHighestHi(long highestHi)
+   {
+      HiLoKeyGenerator.highestHi = highestHi;
+   }
+
    private final Logger log;
    private final DataSource ds;
    private final long blockSize;
@@ -51,6 +60,7 @@
 
    private TransactionManager tm;
    private String updateHiSql;
+   private String selectHiSql;
 
    public HiLoKeyGenerator(
       DataSource ds,
@@ -58,6 +68,7 @@
       String sequenceColumn,
       String sequenceName,
       String idColumnName,
+      String selectHiSql,
       long blockSize,
       TransactionManager tm
       )
@@ -72,7 +83,10 @@
          " set " +
          idColumnName +
          "=?" +
-         " where " + sequenceColumn + "='" + sequenceName + "'";
+         " where " + sequenceColumn + "='" + sequenceName + "' and " +
+         idColumnName + "=?";
+
+      this.selectHiSql = selectHiSql;
    }
 
    public synchronized Object generateKey()
@@ -102,13 +116,9 @@
             throw new IllegalStateException("Failed to begin a new transaction.");
          }
 
-         lo = highestHi + 1;
-         highestHi += blockSize;
-         hi = highestHi;
-
          try
          {
-            updateTable();
+            doGenerate();
             tm.commit();
          }
          catch(SQLException e)
@@ -149,32 +159,84 @@
       return new Long(lo);
    }
 
-   private void updateTable() throws SQLException
+   private void doGenerate() throws SQLException
    {
+      long curHi;
+      do
+      {
+         curHi = getCurrentHi();
+         lo = curHi + 1;
+         hi = curHi + blockSize;
+      }
+      while(!updateHi(curHi, hi));
+   }
+
+   private long getCurrentHi() throws SQLException
+   {
+      return selectHiSql != null ? selectHi() : getHighestHi();
+   }
+
+   private boolean updateHi(long curHi, long newHi) throws SQLException
+   {
+      if(selectHiSql == null)
+      {
+         setHighestHi(newHi);
+      }
+      return updateTable(curHi, newHi);
+   }
+
+   private long selectHi() throws SQLException
+   {
       Connection con = null;
-      PreparedStatement updateHi = null;
+      PreparedStatement selectHiSt = null;
+      ResultSet rs = null;
 
-      if(log.isDebugEnabled())
+      if(log.isTraceEnabled())
       {
-         log.debug("Executing SQL: " + updateHiSql + ", [" + highestHi + "]");
+         log.trace("Executing SQL: " + selectHiSql);
       }
 
       try
       {
          con = ds.getConnection();
-         updateHi = con.prepareStatement(updateHiSql);
-         updateHi.setLong(1, highestHi);
-         final int i = updateHi.executeUpdate();
-
-         if(i != 1)
+         selectHiSt = con.prepareStatement(selectHiSql);
+         rs = selectHiSt.executeQuery();
+         if(!rs.next())
          {
-            throw new SQLException("Expected one updated row but got: " + i);
+            throw new IllegalStateException("The sequence has not been initialized in the service start phase!");
          }
+         return rs.getLong(1);
       }
       finally
       {
+         JDBCUtil.safeClose(rs);
+         JDBCUtil.safeClose(selectHiSt);
+         JDBCUtil.safeClose(con);
+      }
+   }
+
+   private boolean updateTable(long curHi, long newHi) throws SQLException
+   {
+      Connection con = null;
+      PreparedStatement updateHi = null;
+
+      if(log.isTraceEnabled())
+      {
+         log.trace("Executing SQL: " + updateHiSql + ", [" + newHi + "," + curHi + "]");
+      }
+
+      try
+      {
+         con = ds.getConnection();
+         updateHi = con.prepareStatement(updateHiSql);
+         updateHi.setLong(1, newHi);
+         updateHi.setLong(2, curHi);
+         return updateHi.executeUpdate() == 1;
+      }
+      finally
+      {
          JDBCUtil.safeClose(updateHi);
          JDBCUtil.safeClose(con);
       }
    }
-}
+}
\ No newline at end of file

Modified: branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGeneratorFactory.java
===================================================================
--- branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGeneratorFactory.java	2006-10-25 21:23:32 UTC (rev 57834)
+++ branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGeneratorFactory.java	2006-10-25 21:43:10 UTC (rev 57835)
@@ -26,7 +26,7 @@
 import org.jboss.ejb.plugins.keygenerator.KeyGenerator;
 import org.jboss.ejb.plugins.cmp.jdbc.JDBCUtil;
 import org.jboss.ejb.plugins.cmp.jdbc.SQLUtil;
-import org.jboss.util.naming.Util;
+import org.jboss.naming.Util;
 import org.jboss.deployment.DeploymentException;
 
 import javax.management.ObjectName;
@@ -63,8 +63,12 @@
    private String sequenceName;
    private String idColumnName;
    private String createTableDdl;
+   private String selectHiSql;
    private long blockSize;
 
+   private boolean createTable = true;
+   private boolean dropTable;
+
    /**
     * @jmx.managed-attribute
     */
@@ -117,7 +121,6 @@
    {
       if(getState() == STARTED && !tableName.equals(this.tableName))
       {
-         //dropTableIfExists(this.tableName);
          initSequence(tableName, sequenceColumn, sequenceName, idColumnName);
       }
       this.tableName = tableName;
@@ -190,6 +193,22 @@
    /**
     * @jmx.managed-attribute
     */
+   public String getSelectHiSql()
+   {
+      return selectHiSql;
+   }
+
+   /**
+    * @jmx.managed-attribute
+    */
+   public void setSelectHiSql(String selectHiSql)
+   {
+      this.selectHiSql = selectHiSql;
+   }
+
+   /**
+    * @jmx.managed-attribute
+    */
    public long getBlockSize()
    {
       return blockSize;
@@ -203,11 +222,43 @@
       this.blockSize = blockSize;
    }
 
+   /**
+    * @jmx.managed-attribute
+    */
+   public boolean isCreateTable()
+   {
+      return createTable;
+   }
+
+   /**
+    * @jmx.managed-attribute
+    */
+   public void setCreateTable(boolean createTable)
+   {
+      this.createTable = createTable;
+   }
+
+   /**
+    * @jmx.managed-attribute
+    */
+   public boolean isDropTable()
+   {
+      return dropTable;
+   }
+
+   /**
+    * @jmx.managed-attribute
+    */
+   public void setDropTable(boolean dropTable)
+   {
+      this.dropTable = dropTable;
+   }
+
    // KeyGeneratorFactory implementation
 
    public KeyGenerator getKeyGenerator() throws Exception
    {
-      return new HiLoKeyGenerator(ds, tableName, sequenceColumn, sequenceName, idColumnName, blockSize, tm);
+      return new HiLoKeyGenerator(ds, tableName, sequenceColumn, sequenceName, idColumnName, selectHiSql, blockSize, tm);
    }
 
    // ServiceMBeanSupport overrides
@@ -223,13 +274,17 @@
       tm = (TransactionManager)ctx.lookup("java:/TransactionManager");
 
       ds = lookupDataSource(dataSource);
-      dropTableIfExists(tableName);
       initSequence(tableName, sequenceColumn, sequenceName, idColumnName);
    }
 
    public void stopService()
       throws Exception
    {
+      if(dropTable)
+      {
+         dropTableIfExists(tableName);
+      }
+
       ds = null;
       tm = null;
 
@@ -243,14 +298,17 @@
    private void initSequence(String tableName, String sequenceColumn, String sequenceName, String idColumnName)
       throws SQLException, DeploymentException
    {
-      createTableIfNotExists(tableName);
+      if(createTable)
+      {
+         createTableIfNotExists(tableName);
+      }
 
       Connection con = null;
       Statement st = null;
       ResultSet rs = null;
       try
       {
-         String sql = "select * from " + tableName + " where " + sequenceColumn + "='" + sequenceName + "'";
+         String sql = "select " + idColumnName + " from " + tableName + " where " + sequenceColumn + "='" + sequenceName + "'";
          log.debug("Executing SQL: " + sql);
 
          con = ds.getConnection();
@@ -281,6 +339,10 @@
                JDBCUtil.safeClose(insertSt);
             }
          }
+         else
+         {
+            HiLoKeyGenerator.setHighestHi(rs.getLong(1));
+         }
       }
       finally
       {

Modified: branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGeneratorFactoryMBean.java
===================================================================
--- branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGeneratorFactoryMBean.java	2006-10-25 21:23:32 UTC (rev 57834)
+++ branches/JBoss_4_0_4_GA_CP/server/src/main/org/jboss/ejb/plugins/keygenerator/hilo/HiLoKeyGeneratorFactoryMBean.java	2006-10-25 21:43:10 UTC (rev 57835)
@@ -24,43 +24,52 @@
 /**
  * MBean interface.
  */
-public interface HiLoKeyGeneratorFactoryMBean extends org.jboss.system.ServiceMBean
-{
+public interface HiLoKeyGeneratorFactoryMBean extends org.jboss.system.ServiceMBean {
 
    //default object name
-   public static final javax.management.ObjectName OBJECT_NAME = org.jboss.mx.util.ObjectNameFactory
-         .create("jboss.system:service=KeyGeneratorFactory,type=HiLo");
+   public static final javax.management.ObjectName OBJECT_NAME = org.jboss.mx.util.ObjectNameFactory.create("jboss.system:service=KeyGeneratorFactory,type=HiLo");
 
-   void setFactoryName(java.lang.String factoryName);
+  void setFactoryName(java.lang.String factoryName) ;
 
-   java.lang.String getFactoryName();
+  java.lang.String getFactoryName() ;
 
-   void setDataSource(javax.management.ObjectName dataSource) throws java.lang.Exception;
+  void setDataSource(javax.management.ObjectName dataSource) throws java.lang.Exception;
 
-   javax.management.ObjectName getDataSource();
+  javax.management.ObjectName getDataSource() ;
 
-   java.lang.String getTableName();
+  java.lang.String getTableName() ;
 
-   void setTableName(java.lang.String tableName) throws java.lang.Exception;
+  void setTableName(java.lang.String tableName) throws java.lang.Exception;
 
-   java.lang.String getSequenceColumn();
+  java.lang.String getSequenceColumn() ;
 
-   void setSequenceColumn(java.lang.String sequenceColumn);
+  void setSequenceColumn(java.lang.String sequenceColumn) ;
 
-   java.lang.String getSequenceName();
+  java.lang.String getSequenceName() ;
 
-   void setSequenceName(java.lang.String sequenceName);
+  void setSequenceName(java.lang.String sequenceName) ;
 
-   java.lang.String getIdColumnName();
+  java.lang.String getIdColumnName() ;
 
-   void setIdColumnName(java.lang.String idColumnName);
+  void setIdColumnName(java.lang.String idColumnName) ;
 
-   java.lang.String getCreateTableDdl();
+  java.lang.String getCreateTableDdl() ;
 
-   void setCreateTableDdl(java.lang.String createTableDdl);
+  void setCreateTableDdl(java.lang.String createTableDdl) ;
 
-   long getBlockSize();
+  java.lang.String getSelectHiSql();
 
-   void setBlockSize(long blockSize);
+  void setSelectHiSql(String selectHiSql);
 
+  long getBlockSize() ;
+
+  void setBlockSize(long blockSize) ;
+
+   boolean isCreateTable();
+
+   void setCreateTable(boolean createTable);
+
+   boolean isDropTable();
+
+   void setDropTable(boolean dropTable);
 }

Modified: branches/JBoss_4_0_4_GA_CP/server/src/resources/uuid-key-generator/META-INF/jboss-service.xml
===================================================================
--- branches/JBoss_4_0_4_GA_CP/server/src/resources/uuid-key-generator/META-INF/jboss-service.xml	2006-10-25 21:23:32 UTC (rev 57834)
+++ branches/JBoss_4_0_4_GA_CP/server/src/resources/uuid-key-generator/META-INF/jboss-service.xml	2006-10-25 21:43:10 UTC (rev 57835)
@@ -11,7 +11,6 @@
   <!-- UUIDKeyGeneratorFactoryService -->
   <mbean code="org.jboss.ejb.plugins.keygenerator.uuid.UUIDKeyGeneratorFactoryService"
          name="jboss:service=KeyGeneratorFactory,type=UUID">
-     <depends>jboss:service=Naming</depends>
   </mbean>
 
   <!-- HiLoKeyGeneratorFactory -->
@@ -43,6 +42,18 @@
         )
      </attribute>
 
+     <!-- Uncomment to make it cluster-safe: Select current Hi value query (FOR UPDATE is recommended)
+     <attribute name="SelectHiSql">
+        select HIGHVALUES from HILOSEQUENCES where SEQUENCENAME='general' FOR UPDATE
+     </attribute>                                                  
+     -->
+
+     <!-- whether the table should be created (if doesn't exist yet) at the start phase -->
+     <attribute name="CreateTable">true</attribute>
+
+     <!-- whether the table should be dropped (if exists) at the stop phase -->
+     <attribute name="DropTable">false</attribute>
+
      <!-- Instance-specific attributes -->
 
      <!-- JNDI name -->
@@ -53,7 +64,7 @@
 
      <!-- Block size -->
      <attribute name="BlockSize">10</attribute>
-     
   </mbean>
 
 </server>
+




More information about the jboss-cvs-commits mailing list