[#12048] V9 migration and verification script optimization (#12896)

* Add changes * Add changes for migration * Revert the illegals * Linting * Add additional batching * Add notes * Fix linting errors --------- Co-authored-by: Zhang Ziqing <[email protected]>
TEAMMATES · Mar 14, 2024 · 2342189 · 2342189
1 parent 931dea4
commit 2342189
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 18 deletions.
diff --git a/src/client/java/teammates/client/scripts/sql/DataMigrationEntitiesBaseScriptSql.java b/src/client/java/teammates/client/scripts/sql/DataMigrationEntitiesBaseScriptSql.java
@@ -43,15 +43,17 @@ public abstract class DataMigrationEntitiesBaseScriptSql<
         E extends teammates.storage.entity.BaseEntity, T extends teammates.storage.sqlentity.BaseEntity>
         extends DatastoreClient {
 
+    /* NOTE
+     * Before running the migration, please enable hibernate.jdbc.batch_size, hibernate.order_updates,
+     * hibernate.batch_versioned_data, hibernate.jdbc.fetch_size in HibernateUtil.java
+     * for optimized batch-insertion, batch-update and batch-fetching. Also, verify that your schema
+     * meets the conditions for them.
+    */
+
     // the folder where the cursor position and console output is saved as a file
     private static final String BASE_LOG_URI = "src/client/java/teammates/client/scripts/log/";
 
-    // 100 is the optimal batch size as there won't be too much time interval
-    // between read and save (if transaction is not used)
-    // cannot set number greater than 300
-    // see
-    // https://stackoverflow.com/questions/41499505/objectify-queries-setting-limit-above-300-does-not-work
-    private static final int BATCH_SIZE = 100;
+    private static final int BATCH_SIZE = 1000;
 
     // Creates the folder that will contain the stored log.
     static {
@@ -249,6 +251,7 @@ private void flushEntitiesSavingBuffer() {
         if (!entitiesSavingBuffer.isEmpty() && !isPreview()) {
             log("Saving entities in batch..." + entitiesSavingBuffer.size());
 
+            long startTime = System.currentTimeMillis();
             HibernateUtil.beginTransaction();
             for (T entity : entitiesSavingBuffer) {
                 HibernateUtil.persist(entity);
@@ -257,6 +260,8 @@ private void flushEntitiesSavingBuffer() {
             HibernateUtil.flushSession();
             HibernateUtil.clearSession();
             HibernateUtil.commitTransaction();
+            long endTime = System.currentTimeMillis();
+            log("Flushing " + entitiesSavingBuffer.size() + " took " + (endTime - startTime) + " milliseconds");
         }
         entitiesSavingBuffer.clear();
     }

diff --git a/...ient/java/teammates/client/scripts/sql/DataMigrationForAccountAndReadNotificationSql.java b/...ient/java/teammates/client/scripts/sql/DataMigrationForAccountAndReadNotificationSql.java
@@ -141,11 +141,11 @@ protected void migrateEntity(teammates.storage.entity.Account oldAccount) {
 
     private void migrateReadNotification(teammates.storage.entity.Account oldAccount,
             teammates.storage.sqlentity.Account newAccount) {
+
+        HibernateUtil.beginTransaction();
         for (Map.Entry<String, Instant> entry : oldAccount.getReadNotifications().entrySet()) {
-            HibernateUtil.beginTransaction();
             UUID notificationId = UUID.fromString(entry.getKey());
             Notification newNotification = HibernateUtil.get(Notification.class, notificationId);
-            HibernateUtil.commitTransaction();
 
             // If the notification does not exist in the new database
             if (newNotification == null) {
@@ -155,6 +155,7 @@ private void migrateReadNotification(teammates.storage.entity.Account oldAccount
             ReadNotification newReadNotification = new ReadNotification(newAccount, newNotification);
             entitiesReadNotificationSavingBuffer.add(newReadNotification);
         }
+        HibernateUtil.commitTransaction();
     }
 
     @Override

diff --git a/src/client/java/teammates/client/scripts/sql/VerifyNonCourseEntityAttributesBaseScript.java b/src/client/java/teammates/client/scripts/sql/VerifyNonCourseEntityAttributesBaseScript.java
@@ -5,6 +5,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import jakarta.persistence.TypedQuery;
 import jakarta.persistence.criteria.CriteriaBuilder;
@@ -27,7 +28,12 @@ public abstract class VerifyNonCourseEntityAttributesBaseScript<E extends teamma
         T extends teammates.storage.sqlentity.BaseEntity>
         extends DatastoreClient {
 
-    private static int constSqlFetchBaseSize = 500;
+    /* NOTE
+     * Before running the verification, please enable hibernate.jdbc.fetch_size in HibernateUtil.java
+     * for optimized batch-fetching.
+    */
+
+    private static int constSqlFetchBaseSize = 1000;
 
     /** Datastore entity class. */
     protected Class<E> datastoreEntityClass;
@@ -62,10 +68,10 @@ private String getLogPrefix() {
     protected abstract boolean equals(T sqlEntity, E datastoreEntity);
 
     /**
-     * Lookup data store entity.
+     * Lookup data store entities.
      */
-    protected E lookupDataStoreEntity(String datastoreEntityId) {
-        return ofy().load().type(datastoreEntityClass).id(datastoreEntityId).now();
+    protected Map<String, E> lookupDataStoreEntities(List<String> datastoreEntitiesIds) {
+        return ofy().load().type(datastoreEntityClass).ids(datastoreEntitiesIds);
     }
 
     /**
@@ -97,7 +103,7 @@ private List<T> lookupSqlEntitiesByPageNumber(int pageNum) {
         Root<T> root = pageQuery.from(sqlEntityClass);
         pageQuery.select(root);
         List<Order> orderList = new LinkedList<>();
-        orderList.add(cb.asc(root.get("createdAt")));
+        orderList.add(cb.asc(root.get("id")));
         pageQuery.orderBy(orderList);
 
         // perform query with pagination
@@ -117,7 +123,6 @@ protected List<Map.Entry<T, E>> checkAllEntitiesForFailures() {
         // WARNING: failures list might lead to OoM if too many entities,
         // but okay since will fail anyway.
         List<Map.Entry<T, E>> failures = new LinkedList<>();
-
         int numPages = getNumPages();
         if (numPages == 0) {
             log("No entities available for verification");
@@ -128,12 +133,24 @@ protected List<Map.Entry<T, E>> checkAllEntitiesForFailures() {
             log(String.format("Verification Progress %d %%",
                      (int) ((float) currPageNum / (float) numPages * 100)));
 
+            long startTimeForSql = System.currentTimeMillis();
             List<T> sqlEntities = lookupSqlEntitiesByPageNumber(currPageNum);
+            long endTimeForSql = System.currentTimeMillis();
+            log("Querying for SQL for page " + currPageNum + " took "
+                    + (endTimeForSql - startTimeForSql) + " milliseconds");
 
-            for (T sqlEntity : sqlEntities) {
-                String entityId = generateID(sqlEntity);
-                E datastoreEntity = lookupDataStoreEntity(entityId);
+            List<String> datastoreEntitiesIds = sqlEntities.stream()
+                    .map(entity -> generateID(entity)).collect(Collectors.toList());
 
+            long startTimeForDatastore = System.currentTimeMillis();
+            Map<String, E> datastoreEntities = lookupDataStoreEntities(datastoreEntitiesIds);
+            long endTimeForDatastore = System.currentTimeMillis();
+            log("Querying for Datastore for page " + currPageNum + " took "
+                    + (endTimeForDatastore - startTimeForDatastore) + " milliseconds");
+
+            long startTimeForEquals = System.currentTimeMillis();
+            for (T sqlEntity : sqlEntities) {
+                E datastoreEntity = datastoreEntities.get(generateID(sqlEntity));
                 if (datastoreEntity == null) {
                     failures.add(new AbstractMap.SimpleEntry<T, E>(sqlEntity, null));
                     continue;
@@ -145,8 +162,10 @@ protected List<Map.Entry<T, E>> checkAllEntitiesForFailures() {
                     continue;
                 }
             }
+            long endTimeForEquals = System.currentTimeMillis();
+            log("Verifying SQL and Datastore for page " + currPageNum + " took "
+                    + (endTimeForEquals - startTimeForEquals) + " milliseconds");
         }
-
         return failures;
     }
 

diff --git a/src/main/java/teammates/common/util/HibernateUtil.java b/src/main/java/teammates/common/util/HibernateUtil.java
@@ -117,6 +117,11 @@ public static void buildSessionFactory(String dbUrl, String username, String pas
                 .setProperty("hibernate.hbm2ddl.auto", "update")
                 .setProperty("show_sql", "true")
                 .setProperty("hibernate.current_session_context_class", "thread")
+                // Uncomment only during migration for optimized batch-insertion, batch-update, and batch-fetch.
+                // .setProperty("hibernate.jdbc.batch_size", "50")
+                // .setProperty("hibernate.order_updates", "true")
+                // .setProperty("hibernate.batch_versioned_data", "true")
+                // .setProperty("hibernate.jdbc.fetch_size", "50")
                 .addPackage("teammates.storage.sqlentity");
 
         for (Class<? extends BaseEntity> cls : ANNOTATED_CLASSES) {