From b677858388c1de7317faa4eb11ba16bd1e950a4c Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Tue, 29 Aug 2023 10:46:12 -0600 Subject: [PATCH 1/3] Add index on cluster_details.name for FirstFitPlanner speedup Signed-off-by: Marcus Sorensen --- .../upgrade/dao/DatabaseAccessObject.java | 27 ++++++++++++++++++- .../com/cloud/upgrade/dao/DbUpgradeUtils.java | 4 +++ .../upgrade/dao/Upgrade41800to41810.java | 5 ++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java index 21d5a205a09d..242240d4a4ab 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java @@ -18,6 +18,7 @@ import java.sql.Connection; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; import org.apache.log4j.Logger; @@ -84,6 +85,31 @@ public boolean columnExists(Connection conn, String tableName, String columnName return columnExists; } + public void addIndexIfNeeded(Connection conn, String tableName, String columnName) { + boolean indexExists = false; + String indexName = String.format("i_%s__%s", tableName, columnName); + + try (PreparedStatement pstmt = conn.prepareStatement(String.format("SHOW INDEXES FROM %s where Key_name = \"%s\"", tableName, indexName))) { + ResultSet result = pstmt.executeQuery(); + if (result.next()) { + indexExists = true; + } + } catch (SQLException e) { + s_logger.debug(String.format("Index %s doesn't exist, ignoring exception:", indexName, e.getMessage())); + } + + if (indexExists) { + s_logger.debug(String.format("Index %s already exists", indexName)); + } else { + try (PreparedStatement pstmt = conn.prepareStatement(String.format("CREATE INDEX %s on %s (%s)", indexName, tableName, columnName))) { + pstmt.execute(); + s_logger.debug(String.format("Created index %s", indexName)); + } catch (SQLException e) { + s_logger.warn(String.format("Unable to create index %s", indexName), e); + } + } + } + protected static void closePreparedStatement(PreparedStatement pstmt, String errorMessage) { try { if (pstmt != null) { @@ -93,5 +119,4 @@ protected static void closePreparedStatement(PreparedStatement pstmt, String err s_logger.warn(errorMessage, e); } } - } diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java index 02dad6250dcf..3840626a1c6b 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java @@ -23,6 +23,10 @@ public class DbUpgradeUtils { private static DatabaseAccessObject dao = new DatabaseAccessObject(); + public static void addIndex(Connection conn, String tableName, String columnName) { + dao.addIndexIfNeeded(conn, tableName, columnName); + } + public static void addForeignKey(Connection conn, String tableName, String tableColumn, String foreignTableName, String foreignColumnName) { dao.addForeignKey(conn, tableName, tableColumn, foreignTableName, foreignColumnName); } diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41800to41810.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41800to41810.java index 8eb50666ec84..e8835f458dc5 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41800to41810.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41800to41810.java @@ -69,6 +69,7 @@ public void performDataMigration(Connection conn) { copyGuestOsMappingsToVMware80u1(); addForeignKeyToAutoscaleVmprofiles(conn); mergeDuplicateGuestOSes(); + addIndexes(conn); } private void mergeDuplicateGuestOSes() { @@ -242,4 +243,8 @@ private void fixForeignKeyNames(Connection conn) { private void addForeignKeyToAutoscaleVmprofiles(Connection conn) { DbUpgradeUtils.addForeignKey(conn, "autoscale_vmprofiles", "user_data_id", "user_data", "id"); } + + private void addIndexes(Connection conn) { + DbUpgradeUtils.addIndex(conn, "cluster_details", "name"); + } } From c9a7affb8bea9d39507779c79b4bc28db18d41d3 Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Tue, 29 Aug 2023 15:11:55 -0600 Subject: [PATCH 2/3] Add tests to cluster_details index, simplify functions Signed-off-by: Marcus Sorensen --- .../upgrade/dao/DatabaseAccessObject.java | 28 +++++---- .../com/cloud/upgrade/dao/DbUpgradeUtils.java | 10 ++- .../upgrade/dao/Upgrade41800to41810.java | 2 +- .../upgrade/dao/DatabaseAccessObjectTest.java | 61 +++++++++++++++++++ 4 files changed, 84 insertions(+), 17 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java index 242240d4a4ab..0b38acb5c21f 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java @@ -85,28 +85,30 @@ public boolean columnExists(Connection conn, String tableName, String columnName return columnExists; } - public void addIndexIfNeeded(Connection conn, String tableName, String columnName) { - boolean indexExists = false; - String indexName = String.format("i_%s__%s", tableName, columnName); + public String generateIndexName(String tableName, String columnName) { + return String.format("i_%s__%s", tableName, columnName); + } + public boolean indexExists(Connection conn, String tableName, String indexName) { try (PreparedStatement pstmt = conn.prepareStatement(String.format("SHOW INDEXES FROM %s where Key_name = \"%s\"", tableName, indexName))) { ResultSet result = pstmt.executeQuery(); if (result.next()) { - indexExists = true; + return true; } } catch (SQLException e) { s_logger.debug(String.format("Index %s doesn't exist, ignoring exception:", indexName, e.getMessage())); } + return false; + } - if (indexExists) { - s_logger.debug(String.format("Index %s already exists", indexName)); - } else { - try (PreparedStatement pstmt = conn.prepareStatement(String.format("CREATE INDEX %s on %s (%s)", indexName, tableName, columnName))) { - pstmt.execute(); - s_logger.debug(String.format("Created index %s", indexName)); - } catch (SQLException e) { - s_logger.warn(String.format("Unable to create index %s", indexName), e); - } + public void createIndex(Connection conn, String tableName, String columnName, String indexName) { + String stmt = String.format("CREATE INDEX %s on %s (%s)", indexName, tableName, columnName); + s_logger.debug("Statement: " + stmt); + try (PreparedStatement pstmt = conn.prepareStatement(stmt)) { + pstmt.execute(); + s_logger.debug(String.format("Created index %s", indexName)); + } catch (SQLException e) { + s_logger.warn(String.format("Unable to create index %s", indexName), e); } } diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java index 3840626a1c6b..7cae787774a8 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java @@ -21,10 +21,14 @@ public class DbUpgradeUtils { - private static DatabaseAccessObject dao = new DatabaseAccessObject(); + private static final DatabaseAccessObject dao = new DatabaseAccessObject(); - public static void addIndex(Connection conn, String tableName, String columnName) { - dao.addIndexIfNeeded(conn, tableName, columnName); + public static void addIndexIfNeeded(Connection conn, String tableName, String columnName) { + String indexName = dao.generateIndexName(tableName, columnName); + + if (!dao.indexExists(conn, tableName, indexName)) { + dao.createIndex(conn, tableName, columnName, indexName); + } } public static void addForeignKey(Connection conn, String tableName, String tableColumn, String foreignTableName, String foreignColumnName) { diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41800to41810.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41800to41810.java index e8835f458dc5..a58d9965259a 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41800to41810.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41800to41810.java @@ -245,6 +245,6 @@ private void addForeignKeyToAutoscaleVmprofiles(Connection conn) { } private void addIndexes(Connection conn) { - DbUpgradeUtils.addIndex(conn, "cluster_details", "name"); + DbUpgradeUtils.addIndexIfNeeded(conn, "cluster_details", "name"); } } diff --git a/engine/schema/src/test/java/com/cloud/upgrade/dao/DatabaseAccessObjectTest.java b/engine/schema/src/test/java/com/cloud/upgrade/dao/DatabaseAccessObjectTest.java index 2b8b2bd8f1cd..7e78c3ec3e9b 100644 --- a/engine/schema/src/test/java/com/cloud/upgrade/dao/DatabaseAccessObjectTest.java +++ b/engine/schema/src/test/java/com/cloud/upgrade/dao/DatabaseAccessObjectTest.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.upgrade.dao; +import static org.mockito.Matchers.startsWith; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.contains; @@ -27,9 +28,11 @@ import java.sql.Connection; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; import org.apache.log4j.Logger; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -49,6 +52,9 @@ public class DatabaseAccessObjectTest { @Mock private Logger loggerMock; + @Mock + private ResultSet resultSetMock; + private final DatabaseAccessObject dao = new DatabaseAccessObject(); @Before @@ -83,6 +89,61 @@ public void testDropKeyWhenConnectionIsNull() throws Exception { dao.dropKey(conn, tableName, key, isForeignKey); } + @Test + public void generateIndexNameTest() { + String indexName = dao.generateIndexName("mytable","mycolumn"); + Assert.assertEquals( "i_mytable__mycolumn", indexName); + } + + @Test + public void indexExistsFalseTest() throws Exception { + when(resultSetMock.next()).thenReturn(false); + when(connectionMock.prepareStatement(startsWith("SHOW INDEXES FROM"))).thenReturn(preparedStatementMock); + when(preparedStatementMock.executeQuery()).thenReturn(resultSetMock); + + Connection conn = connectionMock; + String tableName = "mytable"; + String indexName = "myindex"; + + Assert.assertFalse(dao.indexExists(conn, tableName, indexName)); + verify(connectionMock, times(1)).prepareStatement(anyString()); + verify(preparedStatementMock, times(1)).executeQuery(); + verify(preparedStatementMock, times(1)).close(); + } + + @Test + public void indexExistsTrueTest() throws Exception { + when(resultSetMock.next()).thenReturn(true); + when(connectionMock.prepareStatement(startsWith("SHOW INDEXES FROM"))).thenReturn(preparedStatementMock); + when(preparedStatementMock.executeQuery()).thenReturn(resultSetMock); + + Connection conn = connectionMock; + String tableName = "mytable"; + String indexName = "myindex"; + + Assert.assertTrue(dao.indexExists(conn, tableName, indexName)); + verify(connectionMock, times(1)).prepareStatement(anyString()); + verify(preparedStatementMock, times(1)).executeQuery(); + verify(preparedStatementMock, times(1)).close(); + } + + @Test + public void createIndexTest() throws Exception { + when(connectionMock.prepareStatement(startsWith("CREATE INDEX"))).thenReturn(preparedStatementMock); + when(preparedStatementMock.execute()).thenReturn(true); + + Connection conn = connectionMock; + String tableName = "mytable"; + String columnName = "mycolumn"; + String indexName = "myindex"; + + dao.createIndex(conn, tableName, columnName, indexName); + verify(connectionMock, times(1)).prepareStatement(anyString()); + verify(preparedStatementMock, times(1)).execute(); + verify(preparedStatementMock, times(1)).close(); + verify(loggerMock, times(1)).debug("Created index myindex"); + } + @Test public void testDropKeyWhenTableNameIsNull() throws Exception { SQLException sqlException = new SQLException(); From 4e5cd40497efe82995b423506f56d3f170f9c6ce Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Wed, 30 Aug 2023 09:29:27 -0600 Subject: [PATCH 3/3] Fix unit test --- .../src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java index 7cae787774a8..6b4e1814de0f 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/DbUpgradeUtils.java @@ -21,7 +21,7 @@ public class DbUpgradeUtils { - private static final DatabaseAccessObject dao = new DatabaseAccessObject(); + private static DatabaseAccessObject dao = new DatabaseAccessObject(); public static void addIndexIfNeeded(Connection conn, String tableName, String columnName) { String indexName = dao.generateIndexName(tableName, columnName);