From d570f6bdf34a7bf0ebfafc4d54ce26e7780a1549 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20Czern=C3=BD?= <pczerny@opennebula.io>
Date: Mon, 30 Sep 2024 14:14:37 +0200
Subject: [PATCH]  F #2111: Optimize VM history handling  (#3243)

- oned load only last 2 history records (not the full list)
- Dump all history records only if needed in VirtualMachine::to_xml.
- Dump conforms XML schecam and removes VM template from history records.

Speed up of onevm show command:
  - for small SQLite DB is for VM with 500 histories: 130 ms down to 5 ms
  - for big MySQL DB VM with 687 histories: 1000 ms down to 200 ms

(cherry picked from commit e13c329db44f02cb18205b2d77fc719bee3324f9)
---
 include/VirtualMachine.h     |  14 ++--
 include/VirtualMachinePool.h |  10 +++
 src/vm/History.cc            |   5 --
 src/vm/VirtualMachine.cc     | 121 ++++++++++++++---------------------
 src/vm/VirtualMachinePool.cc |  13 ++++
 5 files changed, 78 insertions(+), 85 deletions(-)

diff --git a/include/VirtualMachine.h b/include/VirtualMachine.h
index 2877f8fa4d..eaa0a814a3 100644
--- a/include/VirtualMachine.h
+++ b/include/VirtualMachine.h
@@ -413,6 +413,11 @@ public:
     // ------------------------------------------------------------------------
     // History
     // ------------------------------------------------------------------------
+    /*
+     *  Loads all VM history recordsfrom the database
+    */
+    int load_history(SqlDB * db);
+
     /**
      *  Adds a new history record an writes it in the database.
      */
@@ -1835,17 +1840,12 @@ private:
     /**
      *  History record, for the current host
      */
-    History *   history;
+    std::unique_ptr<History> history;
 
     /**
      *  History record, for the previous host
      */
-    History *   previous_history;
-
-    /**
-     *  Complete set of history records for the VM
-     */
-    std::vector<History *> history_records;
+    std::unique_ptr<History> previous_history;
 
     /**
      *  VirtualMachine disks
diff --git a/include/VirtualMachinePool.h b/include/VirtualMachinePool.h
index 7262f44c6c..eeb70af656 100644
--- a/include/VirtualMachinePool.h
+++ b/include/VirtualMachinePool.h
@@ -374,6 +374,16 @@ public:
      */
     VirtualMachineMonitorInfo get_monitoring(int vmid);
 
+    /**
+     *  Dumps the VM history records in XML format
+     *
+     *  @param oss the output stream to dump the pool contents
+     *  @param vid the Virtual Machine ID
+     *
+     *  @return 0 on success
+     */
+    int dump_history(std::string& oss, int vid);
+
     /**
      * Processes all the history records, and stores the monthly cost for each
      * VM
diff --git a/src/vm/History.cc b/src/vm/History.cc
index 41d3a68c6b..4832ca16db 100644
--- a/src/vm/History.cc
+++ b/src/vm/History.cc
@@ -244,11 +244,6 @@ int History::select(SqlDB * db)
         rc = -1;
     }
 
-    if ( rc == 0 ) // Regenerate non-persistent data
-    {
-        non_persistent_data();
-    }
-
     return rc;
 }
 
diff --git a/src/vm/VirtualMachine.cc b/src/vm/VirtualMachine.cc
index 68d07fe717..0e9f46c3ab 100644
--- a/src/vm/VirtualMachine.cc
+++ b/src/vm/VirtualMachine.cc
@@ -56,8 +56,6 @@ VirtualMachine::VirtualMachine(int           id,
     stime(time(0)),
     etime(0),
     deploy_id(""),
-    history(0),
-    previous_history(0),
     disks(false),
     nics(false),
     _log(0),
@@ -82,11 +80,6 @@ VirtualMachine::VirtualMachine(int           id,
 
 VirtualMachine::~VirtualMachine()
 {
-    for (unsigned int i=0 ; i < history_records.size() ; i++)
-    {
-        delete history_records[i];
-    }
-
     delete _log;
 }
 
@@ -663,11 +656,12 @@ int VirtualMachine::bootstrap(SqlDB * db)
 
 int VirtualMachine::select(SqlDB * db)
 {
+
     ostringstream   oss;
     ostringstream   ose;
 
     int    rc;
-    int    last_seq;
+    int    seq;
 
     Nebula& nd = Nebula::instance();
 
@@ -679,36 +673,15 @@ int VirtualMachine::select(SqlDB * db)
         return rc;
     }
 
-    //Get History Records.
-    if ( hasHistory() )
+    if (history && history->select(db) != 0)
     {
-        last_seq = history->seq;
-
-        delete history_records[last_seq];
-
-        for (int i = last_seq; i >= 0; i--)
-        {
-            History * hp;
-
-            hp = new History(oid, i);
-            history_records[i] = hp;
-
-            rc = hp->select(db);
-
-            if ( rc != 0)
-            {
-                goto error_previous_history;
-            }
-
-            if ( i == last_seq )
-            {
-                history = hp;
-            }
-            else if ( i == last_seq - 1 )
-            {
-                previous_history = hp;
-            }
-        }
+        seq = history->seq;
+        goto error_common;
+    }
+    if (previous_history && previous_history->select(db) != 0)
+    {
+        seq = previous_history->seq;
+        goto error_common;
     }
 
     if ( state == DONE ) //Do not recreate dirs. They may be deleted
@@ -768,12 +741,10 @@ int VirtualMachine::select(SqlDB * db)
 
     return 0;
 
-error_previous_history:
-    ose << "Cannot get previous history record (seq:" << history->seq
-        << ") for VM id: " << oid;
+error_common:
+    ose << "Error loading history for VM " << oid  << " seq " << seq;
 
     log("ONE", Log::ERROR, ose);
-
     NebulaLog::error("ONE", ose.str());
 
     return -1;
@@ -2057,7 +2028,7 @@ void VirtualMachine::add_history(
     int           seq;
     string        vm_xml;
 
-    if (history == 0)
+    if (!history)
     {
         seq = 0;
     }
@@ -2065,15 +2036,13 @@ void VirtualMachine::add_history(
     {
         seq = history->seq + 1;
 
-        previous_history = history;
+        previous_history = move(history);
     }
 
     to_xml_extended(vm_xml, 0, false);
 
-    history = new History(oid, seq, hid, hostname, cid, vmm_mad, tm_mad, ds_id,
+    history = make_unique<History>(oid, seq, hid, hostname, cid, vmm_mad, tm_mad, ds_id,
                           vm_xml);
-
-    history_records.push_back(history);
 };
 
 /* -------------------------------------------------------------------------- */
@@ -2081,7 +2050,6 @@ void VirtualMachine::add_history(
 
 void VirtualMachine::cp_history()
 {
-    History * htmp;
     string    vm_xml;
 
     if (history == 0)
@@ -2091,7 +2059,7 @@ void VirtualMachine::cp_history()
 
     to_xml_extended(vm_xml, 0, false);
 
-    htmp = new History(oid,
+    auto htmp = make_unique<History>(oid,
                        history->seq + 1,
                        history->hid,
                        history->hostname,
@@ -2101,10 +2069,8 @@ void VirtualMachine::cp_history()
                        history->ds_id,
                        vm_xml);
 
-    previous_history = history;
-    history          = htmp;
-
-    history_records.push_back(history);
+    previous_history = move(history);
+    history          = move(htmp);
 }
 
 /* -------------------------------------------------------------------------- */
@@ -2112,17 +2078,16 @@ void VirtualMachine::cp_history()
 
 void VirtualMachine::cp_previous_history()
 {
-    History * htmp;
     string    vm_xml;
 
-    if ( previous_history == 0 || history == 0)
+    if ( !previous_history || !history )
     {
         return;
     }
 
     to_xml_extended(vm_xml, 0, false);
 
-    htmp = new History(oid,
+    auto htmp = make_unique<History>(oid,
                        history->seq + 1,
                        previous_history->hid,
                        previous_history->hostname,
@@ -2132,10 +2097,8 @@ void VirtualMachine::cp_previous_history()
                        previous_history->ds_id,
                        vm_xml);
 
-    previous_history = history;
-    history          = htmp;
-
-    history_records.push_back(history);
+    previous_history = move(history);
+    history          = move(htmp);
 }
 
 /* -------------------------------------------------------------------------- */
@@ -2548,7 +2511,6 @@ string& VirtualMachine::to_xml_extended(string& xml, int n_history, bool sa) con
 {
     string template_xml;
     string user_template_xml;
-    string history_xml;
     string perm_xml;
     string snap_xml;
     string bck_xml;
@@ -2590,21 +2552,36 @@ string& VirtualMachine::to_xml_extended(string& xml, int n_history, bool sa) con
 
     if ( hasHistory() && n_history > 0 )
     {
-        oss << "<HISTORY_RECORDS>";
+        string history_xml;
 
         if ( n_history == 2 )
         {
-            for (unsigned int i=0; i < history_records.size(); i++)
+            if (history && history->seq > 1)
             {
-                oss << history_records[i]->to_xml(history_xml);
+                // Dump the full history record
+                auto vmpool = Nebula::instance().get_vmpool();
+                vmpool->dump_history(history_xml, oid);
+
+                // Remove the VM from the history to reduce size and comply with xml-schema
+                ObjectXML obj(history_xml);
+                obj.remove_nodes("/HISTORY_RECORDS/HISTORY/VM");
+
+                oss << obj;
+            }
+            else
+            {
+                oss << "<HISTORY_RECORDS>";
+                if (previous_history) oss << previous_history->to_xml(history_xml);
+                if (history) oss << history->to_xml(history_xml);
+                oss << "</HISTORY_RECORDS>";
             }
         }
         else
         {
+            oss << "<HISTORY_RECORDS>";
             oss << history->to_xml(history_xml);
+            oss << "</HISTORY_RECORDS>";
         }
-
-        oss << "</HISTORY_RECORDS>";
     }
     else
     {
@@ -2733,7 +2710,7 @@ string& VirtualMachine::to_xml_short(string& xml)
     if ( hasHistory() )
     {
         oss << "<HISTORY_RECORDS>";
-        oss << history_records[history_records.size() - 1]->to_xml_short(history_xml);
+        oss << history->to_xml_short(history_xml);
         oss << "</HISTORY_RECORDS>";
     }
     else
@@ -2869,16 +2846,14 @@ int VirtualMachine::from_xml(const string &xml_str)
     if ( xpath(last_seq, "/VM/HISTORY_RECORDS/HISTORY/SEQ", -1) == 0 &&
          last_seq != -1 )
     {
-        history_records.resize(last_seq + 1);
+        history = make_unique<History>(oid, last_seq);
 
-        for (int i=0; i <= last_seq; ++i)
+        // Initialize previous history
+        --last_seq;
+        if (last_seq >= 0)
         {
-            history_records[i] = 0;
+            previous_history = make_unique<History>(oid, last_seq);
         }
-
-        history = new History(oid, last_seq);
-
-        history_records[last_seq] = history;
     }
 
     // -------------------------------------------------------------------------
diff --git a/src/vm/VirtualMachinePool.cc b/src/vm/VirtualMachinePool.cc
index 3289588f81..6b4057a059 100644
--- a/src/vm/VirtualMachinePool.cc
+++ b/src/vm/VirtualMachinePool.cc
@@ -452,6 +452,19 @@ int VirtualMachinePool::dump_monitoring(
 /* -------------------------------------------------------------------------- */
 /* -------------------------------------------------------------------------- */
 
+int VirtualMachinePool::dump_history(std::string& oss, int vid)
+{
+    ostringstream cmd;
+
+    cmd << "SELECT body FROM " << one_db::history_table
+        << " WHERE vid = " << vid << " ORDER BY seq ASC";
+
+    return PoolSQL::dump(oss, "HISTORY_RECORDS", cmd);
+}
+
+/* -------------------------------------------------------------------------- */
+/* -------------------------------------------------------------------------- */
+
 VirtualMachineMonitorInfo VirtualMachinePool::get_monitoring(int vmid)
 {
     ostringstream cmd;